-
Notifications
You must be signed in to change notification settings - Fork 971
Rename fn_args_layout -> fn_params_layout
#5387
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
Conversation
|
This would backport #4163 |
cf5cb60 to
d854403
Compare
src/config/config_type.rs
Outdated
| fn set_fn_args_layout(&mut self) { | ||
| if self.was_set().fn_args_layout() { | ||
| eprintln!( | ||
| "Warning: the `fn_args_layout` option was renamed to `fn_params_layout`." |
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.
Can we also mention that the old option has been deprecated, similar to what we do for other such renames?
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.
Absolutely! I'll make sure to include those details.
Configurations.md
Outdated
| ## `fn_args_layout` | ||
|
|
||
| Control the layout of arguments in a function | ||
| This option has been renamed to `fn_params_layout` to better communicate that it affects the layout of parameters in function signatures. |
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.
Can we also mention that the old option has been deprecated, similar to what we do for other such renames?
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.
I'll update the docs to include the deprecation notice!
fn_args_layout is now deprecated. This option was renamed to better communicate that it affects the layout of parameters in function signatures and not the layout of arguments in function calls. Because the `fn_args_layout` is a stable option the renamed option is also stable, however users who set `fn_args_layout` will get a warning message letting them know that the option has been renamed.
|
Do you think it would be helpful to include some textual description for each variant in the corresponding section on the config docs? The code snippets are fine, but I think a sentence or two that captures behavior which may be non-obvious (e.g. there must be 2+ params for...) would be helpful. |
|
Good call! I'm all for updating the descriptions to include as much detail as possible. I'll make those tweak sometime later this week |
Excellent, thank you. I realize that was an add-on request to the original scope of this PR so I'm going to go ahead and merge. Feel free to submit a follow up with proposed doc enhancements |
fn_args_layoutwas renamedfn_params_layoutto better communicate that it affects the layout of parameters in function signatures and not the layout of arguments in function calls.Because the
fn_args_layoutis a stable option the renamed option is also stable, however users who setfn_args_layoutwill get a warning message letting them know that the option has been renamed.Closes #4149