Skip to content

Rename fn_args_layout config option to fn_params_layout#4163

Merged
topecongiro merged 1 commit intorust-lang:masterfrom
ayazhafiz:i/4149
May 10, 2020
Merged

Rename fn_args_layout config option to fn_params_layout#4163
topecongiro merged 1 commit intorust-lang:masterfrom
ayazhafiz:i/4149

Conversation

@ayazhafiz
Copy link
Copy Markdown
Contributor

See #4149 for more details. This change is targeted for rustfmt 2.0.

Closes #4149

See rust-lang#4149 for more details. This change is targeted for rustfmt 2.0.

Closes rust-lang#4149
@ayazhafiz
Copy link
Copy Markdown
Contributor Author

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!

@calebcartwright
Copy link
Copy Markdown
Member

Thank you for the PR @ayazhafiz!

Does the config option need to be guarded behind a flag

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).

Copy link
Copy Markdown
Member

@calebcartwright calebcartwright left a 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.

Copy link
Copy Markdown
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@karyon
Copy link
Copy Markdown
Contributor

karyon commented Oct 28, 2021

backport not needed, I believe we can't rename stable configs.

@calebcartwright
Copy link
Copy Markdown
Member

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

@ytmimi ytmimi added 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release and removed 1x-backport:n/a labels Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename fn_args_layout config option

5 participants