-
Notifications
You must be signed in to change notification settings - Fork 925
Rename fn_args_layout config option to fn_params_layout #4163
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
See rust-lang#4149 for more details. This change is targeted for rustfmt 2.0. Closes rust-lang#4149
Does the config option need to be guarded behind a flag or will this be cherry-picked for a 2.0 release? If it needs to be behind a flag, I would appreciate some pointers in that 🙂. Thanks! |
Thank you for the PR @ayazhafiz!
Nope, no worries there. It's a breaking change but we'll be including this change in a major version bump as part of the upcoming rustfmt 2.0 release. It would only require a flag if we were releasing it as part of a minor/patch release for rustfmt 1.x (which we won't be doing). |
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.
LGTM, thanks!
@topecongiro - I'll hold off on merging in case you have any thoughts or suggestions on the new config opt name.
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.
LGTM.
backport not needed, I believe we can't rename stable configs. |
We could potentially since there should be a path to a "soft" deprecation where we just print a warning when encountering the option for user awareness, but automatically map the value over to the new option. I think it may be worth reopening #4149 with the extended scope and anyone that wants to work on that could use the commits from here as a building block, and then just add the warning/mapping behavior on top |
See #4149 for more details. This change is targeted for rustfmt 2.0.
Closes #4149