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.

Sign up for GitHub

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

Jump to bottom

chore: add print_stdout/print_stderr lints to workspace level #1425

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: add print_stdout/print_stderr lints to workspace level #1425

wants to merge 1 commit into from

Conversation

oleonardolima
Copy link
Contributor

@oleonardolima oleonardolima commented May 5, 2024 •
edited

potentially fixes #1362

Description

It adds both print_stdout and print_stderr deny level lints on workspace level, but it does allow it on test fns through clippy.toml settings, and explicitly allow it on example code.

Notes to the reviewers

It currently has the setting allowing it on test fns, but open for discussion below.

Changelog notice

  • Add both print_stdout and print_stderr deny level lints on workspace level

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@oleonardolima oleonardolima mentioned this pull request May 5, 2024
Open
@oleonardolima oleonardolima force-pushed the chore/add-lint-to-forbid-print-stdout branch from a02d3c8 to 1524524 Compare May 5, 2024 15:29
@notmandatory notmandatory added the ci label May 5, 2024
@notmandatory
Copy link
Member

This looks like the right way to prevent prints from getting into the code. But do we even want to allow them on tests? The only place I think they should be allowed is in the examples.

@oleonardolima
Copy link
Contributor Author

oleonardolima commented May 6, 2024 •
edited

This looks like the right way to prevent prints from getting into the code. But do we even want to allow them on tests? The only place I think they should be allowed is in the examples.

I'm fine with it either way, but I don't see a problem with allowing it for tests.

@oleonardolima oleonardolima force-pushed the chore/add-lint-to-forbid-print-stdout branch from 1524524 to 9f69ee5 Compare May 6, 2024 20:31
@oleonardolima oleonardolima marked this pull request as ready for review May 7, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oleonardolima oleonardolima

Labels
ci
Projects
BDK
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update CI to prevent merging any prints to stdout
2 participants
@oleonardolima @notmandatory