Skip to content
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

Additional tracing messages to diagnose missing audit criteria. #592

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

anforowicz
Copy link
Contributor

No description provided.

@anforowicz
Copy link
Contributor Author

This PR helped me diagnose problems I've encountered when trying to import https://raw.githubusercontent.com/google/rust-crate-audits/main/manual-sources/google3-audits.toml into Chromium (see Chromium's config.toml here).

PTAL?

@anforowicz
Copy link
Contributor Author

/cc @danakj

@bholley bholley requested a review from mystor February 22, 2024 18:48
src/storage.rs Outdated Show resolved Hide resolved
src/storage.rs Outdated Show resolved Hide resolved
src/storage.rs Outdated Show resolved Hide resolved
@anforowicz anforowicz requested a review from mystor March 26, 2024 23:40
@anforowicz
Copy link
Contributor Author

Thanks for reviewing @mystor. Can you PTAL again?

src/storage.rs Outdated Show resolved Hide resolved
src/storage.rs Outdated Show resolved Hide resolved
src/storage.rs Outdated Show resolved Hide resolved
src/storage.rs Outdated Show resolved Hide resolved
src/storage.rs Outdated
return None;
retain_only_known_criteria(&mut wildcard_entry.criteria, valid_criteria);
if wildcard_entry.criteria.is_empty() {
None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
None
info!("imported wildcard audit parsing failed due to no known criteria");
None

@anforowicz
Copy link
Contributor Author

Thanks for the review @mystor!

Two quick notes:

  • I have a separate PR to clean up Clippy and warnings: Clean up rustc and clippy warnings. #598
  • It seems to me that this PR passes cargo test on CI. My local cargo insta state somehow got messed up and I see cargo test failures locally, but I assume that this is just happening on my machine and shouldn't block this PR.

@mystor
Copy link
Collaborator

mystor commented Apr 2, 2024

It seems to me that this PR passes cargo test on CI. My local cargo insta state somehow got messed up and I see cargo test failures locally, but I assume that this is just happening on my machine and shouldn't block this PR.

This is actually #602 as far as I can tell, and those failures should be fixed now since #603.

@anforowicz
Copy link
Contributor Author

Thanks for the other upstream fixes. FWIW cargo clippy and cargo test pass locally for me, and cargo fmt produces no changes. So maybe let me rebase and squash these changes and this will help make the CI happy?

@mystor mystor merged commit 01227af into mozilla:main Apr 2, 2024
8 checks passed
@mystor
Copy link
Collaborator

mystor commented Apr 2, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants