Introduce --ci flag in tidy#152949
Conversation
This comment has been minimized.
This comment has been minimized.
|
|
|
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
src/tools/tidy/src/lib.rs
Outdated
| ci_env: CiEnv::current(), | ||
| base_commit: None, | ||
| }; | ||
| let ci_env = |
There was a problem hiding this comment.
I'm wondering if we can merge CiInfo into TidyCtx so that TidyCtx becomes the (only) central place to distribute contexts. It also makes the code base simpler.
There was a problem hiding this comment.
Yeah I think it would be worth a try. I remember trying something like that? Maybe, it was a long time ago.. You can try and see if there aren't any issues with that.
src/tools/tidy/src/diagnostics.rs
Outdated
| pub struct TidyFlags { | ||
| /// Applies style and formatting changes during a tidy run. | ||
| bless: bool, | ||
| /// CI flag which for emurating CI env. |
There was a problem hiding this comment.
Not sure what the comment was supposed to say.
There was a problem hiding this comment.
lol, I wanted to say that this flag is for turning on CI mode even in a local environment. I will modify the words 😄
src/tools/tidy/src/lib.rs
Outdated
| ci_env: CiEnv::current(), | ||
| base_commit: None, | ||
| }; | ||
| let ci_env = |
There was a problem hiding this comment.
Yeah I think it would be worth a try. I remember trying something like that? Maybe, it was a long time ago.. You can try and see if there aren't any issues with that.
src/tools/tidy/src/diagnostics.rs
Outdated
| let ci_env = match ci_flag { | ||
| Some(true) => CiEnv::GitHubActions, | ||
| Some(false) => CiEnv::None, | ||
| None => CiEnv::None, |
There was a problem hiding this comment.
| None => CiEnv::None, | |
| None => CiEnv::current(), |
Not that we would run tidy manually on CI, but it should auto detect if the flag is not passed.
There was a problem hiding this comment.
Thanks, I overlooked this misconfiguration. Fixed in de607cc
|
No need to clone @bors delegate |
|
@bors squash |
|
🚧 Squashing... this can take a few minutes. |
* add --ci flag in tidy This commit introduces --ci flag in tidy because currently bootstrap can't pass its ci env information to tidy. It also modifies how CiInfo initialize its ci_env variable. tidy codes which uses CiEnv::is_ci for checking ci are now using ci_env in CiInfo. * address review - Fix comment - Use Option for ci flag in order to have true/false explicitly or unspecified (implicit false) * integrate CiInfo into TidyCtx * remove CiInfo * CiEnv::current() should be called when ci flag is not added * extract base_commit() to a separate function * use &TidyCtx instead of clone
This comment was marked as duplicate.
This comment was marked as duplicate.
5f7b275 to
af29989
Compare
|
🔨 7 commits were squashed into af29989. |
|
Thank you! @bors r+ |
Introduce --ci flag in tidy ## Context Currently tidy doesn't have `--ci` flag. Even if bootstrap has --ci=true, it isn't passed to tidy. As a result, some parts of tidy where we need to turn on `ci mode` are never executed on our local. ## Change This PR introduces --ci flag in tidy, and modify CiInfo and other parts of tidy where currently use `CiEnv::is_ci()` for checking if the running environment is ci or not.
Rollup of 5 pull requests Successful merges: - #152042 (Suggest async block instead of async closure when possible) - #152949 (Introduce --ci flag in tidy) - #152655 (Disable debug_assert_not_in_new_nodes for multiple threads) - #153209 (Clean up `QueryVTable::hash_result` into `hash_value_fn`) - #153229 (rustfmt: add test for field representing type builtin syntax)
Rollup of 5 pull requests Successful merges: - #152042 (Suggest async block instead of async closure when possible) - #152949 (Introduce --ci flag in tidy) - #152655 (Disable debug_assert_not_in_new_nodes for multiple threads) - #153209 (Clean up `QueryVTable::hash_result` into `hash_value_fn`) - #153229 (rustfmt: add test for field representing type builtin syntax)
Rollup merge of #152949 - Shunpoco:tidy-ci, r=Kobzol Introduce --ci flag in tidy ## Context Currently tidy doesn't have `--ci` flag. Even if bootstrap has --ci=true, it isn't passed to tidy. As a result, some parts of tidy where we need to turn on `ci mode` are never executed on our local. ## Change This PR introduces --ci flag in tidy, and modify CiInfo and other parts of tidy where currently use `CiEnv::is_ci()` for checking if the running environment is ci or not.
Context
Currently tidy doesn't have
--ciflag. Even if bootstrap has --ci=true, it isn't passed to tidy. As a result, some parts of tidy where we need to turn onci modeare never executed on our local.Change
This PR introduces --ci flag in tidy, and modify CiInfo and other parts of tidy where currently use
CiEnv::is_ci()for checking if the running environment is ci or not.