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

Use log crate for debugging output in c2rust-analyze #1102

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

ahomescu
Copy link
Contributor

@ahomescu ahomescu commented Jul 6, 2024

Replace all eprintln! macros with uses of the log crate for more efficient and fine-grained debugging output.

@ahomescu ahomescu force-pushed the ahomescu/analyze-logging branch from d4774f4 to 5b143f5 Compare July 6, 2024 06:15
@spernsteiner
Copy link
Collaborator

This is a good idea, but it seems like it will be difficult to merge due to conflicts. Also, it seems like github won't run the CI (or at least doesn't show the CI results here) if it detects conflicts.

I'd suggest running the major CI checks locally first. This is quicker and involves less hassle than running the actual CI, especially given this issue with the conflicts. The major checks are cargo fmt --check (should output nothing; if it outputs a diff, run cargo fmt to apply the suggested changes), cargo check (should give no warnings), and cargo test. For cargo test in particular, you might need to adjust some log levels - a bunch of tests want to inspect the "final labeling for foo" output, which is currently at debug! level in this branch. Or it might be a good idea to let individual tests set their own RUST_LOG values (there's already some machinery for test-specific options that you could extend for this).

Once it's mostly working, you can try doing the procedure of: rebase + test locally + force push + wait for CI + merge. This part of the repo is fairly low-traffic, so I doubt it will be necessary to "close the tree" in order to get this merged.

@kkysen
Copy link
Contributor

kkysen commented Jul 22, 2024

You can also run the CI checks by running ./scripts/run_ci_checks.sh.

Replace all eprintln! macros with uses of the
log crate for more efficient and fine-grained
debugging output.
@thedataking thedataking force-pushed the ahomescu/analyze-logging branch from 5b143f5 to 17b9044 Compare November 27, 2024 03:05
@thedataking thedataking merged commit 3271b3d into master Nov 27, 2024
9 checks passed
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.

4 participants