-
Notifications
You must be signed in to change notification settings - Fork 301
Use debug instead of hidden in debug conf options
#7803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable
Are you sure you want to change the base?
Conversation
debug instead of hidden in debug conf options
| debug | ||
| desc: "A positive epoch selects the epoch at which to stop" | ||
| defaultValue: 0 | ||
| name: "debug-stop-at-epoch" .}: uint64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this contain the debug pragma?
note it's not hidden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a mistaken/an oversight it's not hidden. But the documentation already says it's not supported, effectively, so the better thing to do is mark it hidden.
| debug | ||
| desc: "The wall-time epoch at which to exit the program. (for testing purposes)" | ||
| defaultValue: 0 | ||
| name: "debug-stop-at-epoch" .}: uint64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a hidden stopAtSyncedEpoch below this option. Should that be switched to debug? (also adding debug- prefix).
|
Generally speaking, And, no, |
|
|
||
| trustedSetupFile* {. | ||
| hidden | ||
| debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is/was something to help Ethpandaops run their devnets. That's ... I think? the only use it ever got, and better that it would not be visible at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also regarding this one, I'm not sure it's been used since the Dencun devnets, or has any remaining usecases now that the KZG setup is confirmed/set.
For theoretical-other-KZG setups, they'd be coming with other changes realistically anyway. A parameter to tweak this specifically is likely obsolete even for testing.
|
|
||
| longRangeSync* {. | ||
| hidden | ||
| debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really a "debugging" option, it's a feature that we never ended up enforcing, for various reasons.
etc etc etc
All of these have similar stories. hidden, not debug insofar as it would make them visible with a --help:debug option.
|
|
||
| debugPeerdasSupernode* {. | ||
| hidden | ||
| debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is going away soon anyway, as announced in a recent release notes. It's --debug and that's what --debug-foo options do.
So definitely not something to add visibility to now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this one has been replaced by a supported option, anyway. Turned out there was nontrivial demand due to the blob use cases. So especially useless as a "debug" option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, the key is to set expectations and document the change: 'In release XYZ, --debug-frob has migrated to --frob - the old name will be removed in the 25.x release'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See elsewhere. The point is for this PR that it's a useless thing to expose.
the idea behind it's important that all these options can be discovered using |
|
Special cases become "supported" cases. |
For the cases where this happens, they are migrated to a supported case, with the old option left with a deprecation warning allowing users to migrate too, without breaking their setups unnecessarily - the key is to manage the migration path so that those that use these options understand what's happening. |
|
My actual tendencies for these:
|
Changes:
debug-prefixed conf options fromhiddentodebug. They will show when setting--help:debugNote there are more
hiddenoptions. I only changed the ones that havedebug-prefix.Bump confutils: