-
Notifications
You must be signed in to change notification settings - Fork 974
Introduce required language_edition and style_edition inputs for Diff Check
#6742
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
base: main
Are you sure you want to change the base?
Conversation
|
Just want to note that rustfmt defaults to using |
|
Hm, good point. I'll also make that explicit. |
f04c92f to
b2b5306
Compare
style_edition input for Diff Checklanguage_edition and style_edition inputs for Diff Check
|
Updated the PR to also make language edition explicit. Also rebased since #6741 merged. |
No functional changes. Just was bothering me :D
So that when manually dispatching the Diff Check workflow, an explicit style edition is specified. Note that the style edition specified with `style_edition` input can still be overridden by `rustfmt_configs`, but the intention is to make sure the `style_edition` is always specified.
b2b5306 to
de4e3e7
Compare
fee1-dead
left a comment
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.
had a small nit, otherwise i think good to merge
When a language edition is not specified, `rustfmt` will default to Edition 2015 (like `rustc`), which tends to cause parse errors in the preset set of candidate projects being used for comparison. Introduce a required `language_edition` input for the Diff Check workflow (and the `check_diff.sh` script) so that the contributor invoking the workflow is much less likely to miss specifying the language edition. Like the previous `style_edition` input, the language edition can also be overriden by `edition=...` settings in the later optional `rustfmt_configs` input for consistency.
de4e3e7 to
8c0ebb8
Compare
|
Fixed comment, no other changes. |
So that when manually dispatching the Diff Check workflow, an explicit pair of {language edition, style edition} are specified.
rustfmtwill default to Edition 2015 likerustc, which can cause a lot of parse errors in the current set of candidate projects used for comparingrustfmtbehaviors since they tend to be on Edition 2024.rustfmtcan pick a lower default style edition than the actual language edition, which can also lead to unexpected diff check outcomes.Note that the language/style editions specified with
language_edition/style_editioninputs can still be overridden byrustfmt_configs, but the intention is to make sure thelanguage_edition/style_editionare always specified.Context: #6681 (comment)
Separate
--edition/--style-editionvs--config=edition=...,style_edition=...See discussion in #t-rustfmt > style edition flag vs config.
In this PR I use the
--config=style_edition=$STYLE_EDITIONform, because I'm not actually sure what's the precedence order of mixed invocations (example below), so I just used--config=style_edition=$STYLE_EDITIONwhere laterstyle_editionsspecified in optionalrustfmt_configsshould take precedence over this input.Review remarks
The first commit in this PR is a drive-by fix for
rusfmt->rustfmtthat was bothering me, should not contain any functional changes.The following commits are the actual script/workflow changes.