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

Update CI to prevent merging any prints to stdout #1362

Open
stevenroose opened this issue Feb 28, 2024 · 4 comments · May be fixed by #1425
Open

Update CI to prevent merging any prints to stdout #1362

stevenroose opened this issue Feb 28, 2024 · 4 comments · May be fixed by #1425
Labels
bug Something isn't working module-blockchain
Milestone

Comments

@stevenroose
Copy link
Contributor

                #[cfg(feature = "std")]
                debug_assert!({
                    println!("txid={} skip={}", txid, skip);
                    true
                });

this piece of code in tx_graph.rs prints out stuff when run in dev profile. That's pretty annoying tbh, I don't think it's uncommon for people to use CLIs in the dev profile and this messes up with stdout for these CLIs.

I reckon this has something to do with debugging, but I think there must be better ways to get these kinds of printouts without bothering users?

@stevenroose stevenroose added the bug Something isn't working label Feb 28, 2024
@LLFourn
Copy link
Contributor

LLFourn commented Feb 28, 2024

I think we should add to ci that:

rg 'dbg!|println!' -g '!*tests*' -g '!*examples*' -g '*.rs' crates

Should always show up clean

@ValuedMammal
Copy link
Contributor

This might actually be fixed in #1194

@notmandatory
Copy link
Member

Ya I did remove this debug_assert! in #1194 but if someone wants to remove it sooner in another PR that's fine by me.

@notmandatory notmandatory added this to the 1.0.0-alpha milestone Mar 12, 2024
@nondiremanuel nondiremanuel modified the milestones: 1.0.0-alpha, 1.0.0-beta Mar 21, 2024
@notmandatory notmandatory changed the title bdk tx_graph module prints to stdout when run in dev profile Update CI to prevent merging any prints to stdout Mar 21, 2024
@oleonardolima
Copy link
Contributor

I tried a different approach on #1425, using both print_stdout and print_stderr lints, instead of adding a new step on the CI, wdyt @LLFourn @notmandatory ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working module-blockchain
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

6 participants