Skip to content

Conversation

@0xPoe
Copy link
Collaborator

@0xPoe 0xPoe commented Jan 29, 2024

ref #493 (comment)

Description

This pull request adds a new flag to tokio-console that allows warning lints to be selectively disabled. Additionally, you can use the value all to disable all warnings.
For example, you can use it like this: following manner: cargo run -- --warn "self-wakes,lost-waker,never-yielded" --allow "self-wakes,never-yielded"

Explanation of Changes

Added an AllowedWarnings enum to the command line interface to represent the allowed warnings. All for disable all warnings. Explicit explicitly indicates which warning can be ignored.

@0xPoe 0xPoe changed the title feat: add --allow flag WIP: feat: add --allow flag Jan 29, 2024
Copy link
Collaborator Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

Still working on adding the --allow all support.

@0xPoe
Copy link
Collaborator Author

0xPoe commented Feb 7, 2024

Tested locally:

  1. panic on main.rs with panic!("{:?}", warnings);
  2. Try:
$ cargo run -- --warn "self-wakes" --allow all
     Running `target/debug/tokio-console --warn self-wakes --allow all`
The application panicked (crashed).
Message:  []
Location: tokio-console/src/main.rs:79
  1. Another one:
$ cargo run -- --warn "self-wakes" --allow ohno
error: invalid value 'ohno' for '--allow <ALLOW_WARNINGS>...': failed to parse warning: unknown warning: ohno

For more information, try '--help'.
  1. Comma list:
$ cargo run -- --warn "self-wakes,lost-waker,never-yielded" --allow "self-wakes,never-yielded"
     Running `target/debug/tokio-console --warn self-wakes,lost-waker,never-yielded --allow self-wakes,never-yielded`
The application panicked (crashed).
Message:  [LostWaker]
Location: tokio-console/src/main.rs:79

Copy link
Collaborator Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

🔢 Self-check

@0xPoe 0xPoe marked this pull request as ready for review February 7, 2024 16:17
@0xPoe 0xPoe requested a review from a team as a code owner February 7, 2024 16:17
@0xPoe 0xPoe changed the title WIP: feat: add --allow flag feat: add --allow flag Feb 7, 2024
Copy link
Collaborator

@hds hds left a comment

Choose a reason for hiding this comment

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

This looks great, just a couple of small things.

0xPoe added 12 commits February 10, 2024 09:41
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
@0xPoe 0xPoe force-pushed the rustin-patch-allow-warnings branch from fe35f43 to e7a610e Compare February 10, 2024 01:41
@0xPoe 0xPoe requested a review from hds February 10, 2024 01:41
Copy link
Collaborator Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

🔢 Self-check

Copy link
Collaborator

@hds hds left a comment

Choose a reason for hiding this comment

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

Looks good. Perhaps just add the comment on AllowedWarnings regarding ValueEnum that Eliza suggested and it's good to go!

Signed-off-by: hi-rustin <[email protected]>
@0xPoe
Copy link
Collaborator Author

0xPoe commented Feb 12, 2024

Perhaps just add the comment on AllowedWarnings regarding ValueEnum that Eliza suggested and it's good to go!

Added.
Thanks for your review! 💚 💙 💜 💛 ❤️

@0xPoe 0xPoe requested a review from hawkw February 12, 2024 16:23
@hds
Copy link
Collaborator

hds commented Feb 12, 2024

Added.
Thanks for your review! 💚 💙 💜 💛 ❤️

Great, thank you! Would you mind adding a bit of a longer description to this MR (we'll then use that as the commit message).

Ideally 2 paragraphs: the first one explaining what the PR is trying to achieve (why are we making the change). The second paragraph should explain at a high level what is being changed.

This then becomes a really valuable resource for the future to understand why certain changes have been made.
Thanks!

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

This looks good to me! Please make sure to update the commit message the way @hds described, and then we'd love to merge this PR!

@0xPoe
Copy link
Collaborator Author

0xPoe commented Feb 13, 2024

Great, thank you! Would you mind adding a bit of a longer description to this MR (we'll then use that as the commit message).

Ideally 2 paragraphs: the first one explaining what the PR is trying to achieve (why are we making the change). The second paragraph should explain at a high level what is being changed.

Added! Thanks for your remainder. I will add it to all my PRs.

@hds hds merged commit 8da7037 into tokio-rs:main Feb 13, 2024
hds pushed a commit that referenced this pull request Jun 10, 2024
…scriber-v0.3.0 (#557)

## 🤖 New release
* `tokio-console`: 0.1.10 -> 0.1.11
* `console-api`: 0.6.0 -> 0.7.0
* `console-subscriber`: 0.2.0 -> 0.3.0


## `tokio-console`

## tokio-console-v0.1.11 - (2024-06-10)

### Added

- Replace target column with kind column in tasks view ([#478](#478)) ([903d9fa](903d9fa))
- Add flags and configurations for warnings ([#493](#493)) ([174883f](174883f))
- Add `--allow` flag ([#513](#513)) ([8da7037](8da7037))

### Documented

- Add note about running on Windows ([#510](#510)) ([a0d20fd](a0d20fd))

### Fixed

- Ignore key release events ([#468](#468)) ([715713a](715713a))
- Accept only `file://`, `http://`, `https://` URI ([#486](#486)) ([031bddd](031bddd))
- Fix column sorting in resources tab ([#491](#491)) ([96c65bd](96c65bd))
- Only trigger lints on async tasks ([#517](#517)) ([4593222](4593222))
- Remove duplicate controls from async ops view ([#519](#519)) ([f28ba4a](f28ba4a))
- Add pretty format for 'last woken' time ([#529](#529)) ([ea11ad8](ea11ad8))


## `console-api`

## console-api-v0.7.0 - (2024-06-10)

### <a id = "0.7.0-breaking"></a>Breaking Changes
- **Bump tonic to 0.11 ([#547](#547 ([ef6816c](https://github.com/tokio-rs/console/commit/ef6816caa0fe84171105b513425506f25d3082af))<br />This is a breaking change for users of `console-api` and
`console-subscriber`, as it changes the public `tonic` dependency to a
semver-incompatible version. This breaks compatibility with `tonic`
0.10.x.

### Documented

- Fix typo in proto ([#472](#472)) ([2dd3559](2dd3559))

### Updated

- [**breaking**](#0.7.0-breaking) Bump tonic to 0.11 ([#547](#547)) ([ef6816c](ef6816c))


## `console-subscriber`

## console-subscriber-v0.3.0 - (2024-06-10)

### <a id = "0.3.0-breaking"></a>Breaking Changes
- **Bump tonic to 0.11 ([#547](#547 ([ef6816c](https://github.com/tokio-rs/console/commit/ef6816caa0fe84171105b513425506f25d3082af))<br />This is a breaking change for users of `console-api` and
`console-subscriber`, as it changes the public `tonic` dependency to a
semver-incompatible version. This breaks compatibility with `tonic`
0.10.x.

### Added

- Replace target column with kind column in tasks view ([#478](#478)) ([903d9fa](903d9fa))
- Reduce retention period to fit in max message size ([#503](#503)) ([bd3dd71](bd3dd71))
- Support grpc-web and add `grpc-web` feature ([#498](#498)) ([4150253](4150253))

### Documented

- Add a grpc-web app example ([#526](#526)) ([4af30f2](4af30f2))
- Fix typo in doc comment ([#543](#543)) ([ff22678](ff22678))

### Fixed

- Don't save poll_ops if no-one is receiving them ([#501](#501)) ([1656c79](1656c79))
- Ignore metadata that is not a span or event ([#554](#554)) ([852a977](852a977))

### Updated

- [**breaking**](#0.3.0-breaking) Bump tonic to 0.11 ([#547](#547)) ([ef6816c](ef6816c))
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.

3 participants