[ConfigManager] Register Node Sections 4/4 - #3995
Draft
bdchatham wants to merge 2 commits into
Draft
Conversation
Fourteen keys sit at the top of that file with no table of their own. They are declared against a schema rather than the node's top-level type, because that type carries the nine tables as well and declaring against it would declare every table's keys a second time. The schema squashes the same base group that type squashes, so those spellings still come from the node's own tags, and it restates the two fields held beside that group. A test holds those two to the type they came from by name, tag and type, and holds the count, so a third one appearing there fails rather than going undeclared. Two paths are left out. The home directory is where this file is found, so a value inside it would be the file naming its own location, and the command line already carries it. The node mode is the fact the file states at the top under its own name, and a second spelling would let the two disagree, with the resolution answering for one and the node reading the other. A test also checks that no root key is another section's name. Nothing refuses that collision, and the two settings it produces cannot both be written because no file holds a value for a name and a table under it. This is the first change to declare root keys beside another package's, so the check lives here until it has somewhere better to be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## plt-775-node-sections-3 #3995 +/- ##
========================================================
Coverage 57.61% 57.61%
========================================================
Files 2230 2230
Lines 187682 187690 +8
========================================================
+ Hits 108126 108134 +8
Misses 69689 69689
Partials 9867 9867
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
# Conflicts: # config/tendermintbase/tendermintbase_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Declares the fourteen keys that sit at the top of the node's configuration file with
no table of their own. That completes the file: nine tables and its root. Nothing
consumes them, so a node's configuration reads exactly as it did.
Why these are declared against a schema
The node's top-level type carries the root keys and the nine tables both, so
declaring against it would declare every table's keys a second time.
The schema here squashes the same base group that type squashes, so fourteen
spellings still come from the node's own tags and cannot drift from them. It
restates only the two fields held beside that group.
Those two restatements are the one place a spelling is written twice, so a test
holds them to the type they came from by name, tag and type, and holds the count of
non-table fields on that type. A third field appearing there fails here rather than
going undeclared and unnoticed.
Two paths left out
homeis where this file is found, so a value inside it would be the file namingits own location, and the command line already carries it.
modeis the fact the file states at the top under its own name. Declaring a secondspelling would let the two disagree, with the resolution answering for one and the
node reading the other.
This leaves the step that makes these keys take effect something to do. Nothing
will carry the file's own mode into the node's
mode, so whatever installs a writtenvalue has to map it. Worth stating here rather than discovering there.
The test names both paths as literals rather than reading the list the registration
uses. A test comparing that list against itself agrees however the list changes, so
a path dropped from it would leave the test passing while the key became declared. I
checked that by removing
modeand confirming the test now objects.A collision nothing refuses
A key at the top of the file that is also a section's name cannot be written. No
file holds both a value for a name and a table under that name, so one of the two
settings is unreachable and nothing says which.
The registry does not catch this. It has no instance today, and this is the first
change to declare root keys beside another package's, so a test here checks the
whole registered set. It belongs somewhere better eventually; a check that exists is
worth more than the right home for it.
Verification
gofmt,goimports,go vetclean.golangci-lintreports nothing on the touchedpackage.
config/...andcmd/seid/...pass.I also broke two behaviours on purpose and checked a test objected: a restated root
field going unchecked, and the mode key being declared after all. Both were caught,
the second only after the self-referential test above was rewritten.
Two notes on what happens after this
The stacked base is behind the four it holds. Those PRs have taken review fixes and a
mainmerge since this base branch was cut, so it is short of them. The diff here is still only this
step's work, because the base is a consistent ancestor. But
mainhas to be merged into thisstack before the first PR is retargeted, or a main-based diff would read as reverting the later
commits on those branches.
One key this leaves for whatever delivers these values.
modeis not declared, so nothinghere can set it, and
sei.tomlstates the same fact at its top under its own name. Those twocan disagree across the two files with nothing saying so, and a node resolving one kind's values
while running as another reads correctly in every other report about it. Comparing them and
reporting a disagreement belongs with the delivery, not here.
Reviewing this
Last of four, stacked on the third, so the diff shows only this step. With it the
registry declares 12 sections and 188 keys.