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

Adding attributes to catch stray debug prints before merging #154

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

MitchellBerend
Copy link
Collaborator

@MitchellBerend MitchellBerend commented Oct 23, 2022

Resolves #148
Resolves #156

This pr does not actually resolve any issue but it will prevent stray debug println from being commited to the main branch in the future.

This pr adds attributes to all the modules imported in src/lib.rs. These attributes prevent stray debug println!s from making it into the main branch.

For future reference, you can annotate your function or method with the following attribute so clippy passes ci:

#[allow(clippy::print_stdout)]
fn your_function() {}

Additional tasks

- [ ] Documentation for changes provided/changed
- [ ] Tests added
- [ ] Updated CHANGELOG.md

@MitchellBerend MitchellBerend added CI blocked This issue is blocked by another issue labels Oct 23, 2022
@MitchellBerend MitchellBerend marked this pull request as draft October 23, 2022 10:44
@MitchellBerend
Copy link
Collaborator Author

@MitchellBerend The issue is about removing extra calls and it's now resolved by @jim4067 in #152.
Could you open a separate issue about clippy warnings so we don't lose this useful info until we improve our DX? I didn't have the chance to look into it yet disappointed
Also, IIUC, it should be enough to put these clippy options only in src/lib.rs to apply them to the entire tool-sync which is not that bad actually. So maybe we can do this instead of patching our CI config thinking
MitchellBerend reacted with thumbs up emoji

@chshersh You were right with this. This needs to be added at the top of src/lib.rs

#![deny(clippy::print_stdout)]

#[deny(clippy::print_stdout)]
mod completion;
#[deny(clippy::print_stdout)]
mod config;
#[deny(clippy::print_stdout)]
mod infra;
#[deny(clippy::print_stdout)]
mod install;
#[deny(clippy::print_stdout)]
mod model;
#[deny(clippy::print_stdout)]
mod sync;

@MitchellBerend MitchellBerend changed the title Added clippy option to precommit and ci Adding attributes to catch stray debug prints before merging Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue is blocked by another issue CI
Projects
None yet
1 participant