-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Consolidate option dumps #18865
Consolidate option dumps #18865
Conversation
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.
Looks good to me 👍
Tested against the new session types and some other modules and everything seems to be working as expected 👍
|
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.
Looks sane to me at a glance; just those open questions/suggestions
Consolidating some duplicate code in the
dump_*_options
and pulling some logic out of PR #18817 since it makes sense to be landed separately and make the other PR easier to reviewValidation steps
options
on a handful of modules check the output is sensibleshow advanced
andshow evasion