Tidy: disallow TODO in other in-tree projects#153166
Tidy: disallow TODO in other in-tree projects#153166rust-bors[bot] merged 3 commits intorust-lang:mainfrom
Conversation
|
Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred in compiler/rustc_codegen_gcc |
|
Tidy does not run inside the cg_clif and cg_gcc repos, which means whenever we do a sync, there is a fair chance that we did be forced to male changes when tidy complains. |
I would suggest not running full Also, doesn't rust-lang/compiler-team#963 only say TODO -> FIXME? Not other |
|
Ah, I overinterpreted it — my mistake. Thanks for the clarification. I’ll update it accordingly. |
This comment has been minimized.
This comment has been minimized.
8af1d2e to
474d3e6
Compare
474d3e6 to
ac17a97
Compare
89a1cb9 to
2617d54
Compare
There was a problem hiding this comment.
I've also added a TODO check to the CI for cg_* (rust-lang/rustc_codegen_cranelift#1632, rust-lang/rustc_codegen_gcc#861).
There was a problem hiding this comment.
I tried to add cg_* checks to style.rs, which checks TODOs in tidy, but since it also checks other things besides TODOs, I separated it into codegen.rs.
| } | ||
| } | ||
|
|
||
| // todo: this function now accepts `Session` instead of `ParseSess` and should be relocated |
This comment has been minimized.
This comment has been minimized.
2617d54 to
a035848
Compare
| .enumerate() | ||
| .map(|(index, field)| { | ||
| self.context.new_field(None, *field, format!("field{}_TODO", index)) | ||
| self.context.new_field(None, *field, format!("field{}_FIXME", index)) |
There was a problem hiding this comment.
Oh, I'll check this again. Thank you
There was a problem hiding this comment.
I changed TODO to FIXME and tested it and it worked.
rust-lang/rustc_codegen_gcc@d82d242
cc: @antoyo
src/tools/tidy/src/style.rs
Outdated
| let is_codegen_tidy_file = file.ends_with(codegen_file); | ||
| if is_codegen_tidy_file { | ||
| return; | ||
| } |
There was a problem hiding this comment.
a035848#diff-2877cff958f18d248fbdb0e5f924a453e3911e7e8340139125911d3bdd8e5a62R49-R53
I'm using the "TODO" character here and top line, this is the part that was added to skip TODO check
There was a problem hiding this comment.
we already do it for this_file separately, can you the checks for these two?
after that r=me
a035848 to
e09936e
Compare
|
thank you for the review |
|
@reddevilmidzy: 🔑 Insufficient privileges: not in review users |
|
@lcnr It's not working because I don't have permission... haha, Please roll it up. |
|
Though I'm not a compiler team member, I have the permission and it would be okay for me to add this to queue as lcnr said the following 😅
@bors r=lcnr |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing d27207d (parent) -> 1e21831 (this PR) Test differencesShow 2 test diffs2 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 1e2183119f0ee19cc26df899e26b04ad0de3475d --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (1e21831): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -3.5%, secondary -2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResults (secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 481.726s -> 481.243s (-0.10%) |
Fixes: #152280
MCP: rust-lang/compiler-team#963
TODO
cg_clif: Add TODO checker to cg_clif rustc_codegen_cranelift#1632cg_gcc: Add TODO checker to cg_gcc rustc_codegen_gcc#861r? lcnr