-
Notifications
You must be signed in to change notification settings - Fork 13.7k
tidy: check that error messages don't start with a capitalized letter #146171
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 comment has been minimized.
This comment has been minimized.
0b8a1dd
to
4d08e6e
Compare
Some changes occurred in compiler/rustc_codegen_ssa |
@rustbot ready |
&mut |ent, contents| { | ||
check_lowercase(ent.path().to_str().unwrap(), contents, bad); | ||
}, |
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.
q: doesn't this get called for dirs? if so, you should probably add a check for them here.
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.
it is not called on dirs because we filter out dirs right before that:
|path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)),
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.
that's not what filter_dirs
does though? (otherwise you'd just have !is_dir &&
) (filter_dirs
seems to just ignore common dirs you want to ignore)
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.
that's not what filter_dirs does though?
You're right! My comment was terribly badly worded, sorry :(. (I shouldn't have used words "filter out")
This is not called on dirs due to the !is_dir
part of the lign I quoted, not because of the filter_dirs
bit
But now that you ask, I have to admit there's something fishy with the &&
and ||
in the condition (honestly, I copied this from the other fluent-related checks). I'll try to write down the truth table for this tomorrow.
4d08e6e
to
b152974
Compare
@bors r+ rollup |
Rollup of 8 pull requests Successful merges: - #113095 (Document `become` keyword) - #146159 (Some hygiene doc improvements) - #146171 (tidy: check that error messages don't start with a capitalized letter) - #146419 (Update the arm-* and aarch64-* platform docs.) - #146473 (Revert "Constify SystemTime methods") - #146506 (Fix small typo in check-cfg.md) - #146517 (fix Condvar::wait_timeout docs) - #146521 (document `core::ffi::VaArgSafe`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146171 - scrabsha:push-wovnxxwltsun, r=WaffleLapkin tidy: check that error messages don't start with a capitalized letter
Next step after #146112. The implementation of this is better than the hacks I wrote earlier, which allowed me to discover more capitalized error messages.
@rustbot blocked
r? @WaffleLapkin