-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Simple solution to validate toml in issue #106104 #106559
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jyn514 (or someone else) soon. Please see the contribution instructions for more information. |
This comment has been minimized.
This comment has been minimized.
src/tools/tidy/src/triagebot.rs
Outdated
walk(path, &mut |path| filter_dirs(path), &mut |entry, contents| { | ||
let file = entry.path(); | ||
let filename = file.file_name().unwrap(); | ||
if filename != "triagebot.toml" { |
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.
Maybe only check the top-level triagebot.toml
instead of trying to find it in all subdirectories?
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.
Sure, I'll fix it. I had searched the repo for the triagebot.toml
files and saw multiple, hence I thought maybe I should do a walk. Is there a reason we don't check those toml files?
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.
Only the top-level one is actually used by triagebot afaik. Several subdirectories of src/tools
are submodules or subtrees. Their copy is only used for the separate repo from which they are pulled in. For example src/tools/miri
is a subtree pulled from https://github.com/rust-lang/miri/, so the https://github.com/rust-lang/miri/ repo has a triagebot.toml
file which is used when interacting with this repo, but it is also copied into src/tools/miri/triagebot.toml
as part of git subtree
syncs, as everything is copied.
Sorry, I meant to reply on the issue. I liked Eric's idea of validating this in triagebot, I don't think it should go in tidy. Fortunately I think nearly all of the code you've written can stay the same :) you'd just need to handle posting a comment on the PR if the check fails. |
Got it, I'll get to work on that! Thanks! |
Fixes #106104 using the
toml
crate to validate if thetriagebot.toml
file is in valid toml.