-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Improve options display optional session types #18817
Improve options display optional session types #18817
Conversation
Could we add tests to |
ea70bdf
to
9a4b8b9
Compare
Not a blocker; Looks like we should be updating |
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.
Not a blocker; This existing class extending Hash
is odd. It's probably something we should remove in the future if the API surface area is small :+1;
71868bd
to
09edb84
Compare
be98b6a
to
bbcc763
Compare
@dwelch-r7 Can we squash the commits down here? 🎉 |
74a9842
to
1bf3df2
Compare
Ran through all the new session types modules and everything is working as expected. I think this is much better now in terms off making it clear what is required in order for a user to know what needs to be set depending on what method they use for modules. Great change 📈 |
1bf3df2
to
bf1608a
Compare
Release NotesThis PR adds support to now bucket module options that are output after running the |
Merge after #18770
This PR splits up the output of the
options
command for modules that support either anrhost
or asession
connection to show that only one or the other is required as this was ambiguous beforeBefore:
After:
Verification Steps
options