feat: cleanup only scan managed files#5338
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
a0dfdec to
6d7cdb1
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Hi @westonpace, this PR is ready for review. Please take a look if you have time. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
91faea3 to
355b14e
Compare
355b14e to
bc777d3
Compare
yanghua
left a comment
There was a problem hiding this comment.
Overall, LGTM, only a minor question.
|
Thanks for reviewing, I will wait 2 days to see if there are any other comments. |
westonpace
left a comment
There was a problem hiding this comment.
Overdue review but wanted to thank you for the contribution and good idea.
| }; | ||
|
|
||
| const VERSIONS_DIR: &str = "_versions"; | ||
| pub const VERSIONS_DIR: &str = "_versions"; |
There was a problem hiding this comment.
There's some inconsistency with all these dir constants. Some are here, others are in dataset.rs. I don't think it really matters but it might be nice sometime to just put them all in datasets.rs and either make them all pub (which I think we've done here) or refer to them all from the Dataset accessor functions.
There was a problem hiding this comment.
Thanks for the suggestion.
I was planning to go with the latter approach, but I didn’t want to be too aggressive. I will carry this forward in other PRs
Close #5294