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

Add verbose flag #178

Merged
merged 4 commits into from
May 20, 2024
Merged

Add verbose flag #178

merged 4 commits into from
May 20, 2024

Conversation

wadeduvall
Copy link
Contributor

@wadeduvall wadeduvall commented May 17, 2024

Add a -v/--verbose flag so that kernel version isn't printed every time pacman updates or for MOTD.

@wadeduvall
Copy link
Contributor Author

I'm open to other ways to do this like a --verbose or moving the println!(..) to info!(...) but wanted to make the changes as small as possible.

Copy link
Owner

@rnestler rnestler left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

I agree that the output if nothing needs to be done is unnecessary (I probably had it as a debug info in the beginning and just left it).

I'm open to other ways to do this like a --verbose or moving the println!(..) to info!(...) but wanted to make the changes as small as possible.

Currently I'd slightly prefer to disable the unnecessary output by default and add a --verbose flag.

What is your opinion?

src/kernel.rs Outdated
let should_reboot = running_kernel_version != &cleaned_kernel_version;
if !self.quiet || should_reboot {
Copy link
Owner

Choose a reason for hiding this comment

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

According to https://tldp.org/LDP/abs/html/standard-options.html --quiet is expected to "suppress stdout".

Here we only suppress it if should_reboot is false. But of course this behavior makes a lot of sense given the purpose mentioned by you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's fine. I wasn't sure if there was a reason for that output, so I started off w/ a minimal change. Do you want anything else printed with --verbose?

Copy link
Owner

@rnestler rnestler May 20, 2024

Choose a reason for hiding this comment

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

Do you want anything else printed with --verbose?

We could output some information on the various checkers, but I for myself am mostly interested in the kernel version. I sometimes use it as an alternative to uname -r with my rab alias to check which kernel variant I booted.

So for now I think we are fine with the just having the old behavior when passing --verbose and be more silent by default.

@rnestler rnestler self-assigned this May 20, 2024
@rnestler
Copy link
Owner

What would be nice as well would be a CHANGELOG entry. But I can also do it myself before releasing.

@wadeduvall
Copy link
Contributor Author

What would be nice as well would be a CHANGELOG entry. But I can also do it myself before releasing.

I can take care of that this evening!

package updated if verbose flag is set. Continue to display kernel
version even when an update is not needed.
@wadeduvall
Copy link
Contributor Author

Okay, maybe not exactly how you wanted it (I can adjust if needed) but: if verbose is not set, only notify that a reboot/session reload is requited. Withe verbose flag, print the kernel version (always) and package version (if an update is required). I also added a changelog entry for 1.7.2 with this PR. Thanks for being kind, its my first PR!

@rnestler rnestler changed the title Add quiet flag Add verbose flag May 20, 2024
Copy link
Owner

@rnestler rnestler left a comment

Choose a reason for hiding this comment

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

Some small changes needed.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@rnestler rnestler merged commit 850fdf6 into rnestler:master May 20, 2024
3 checks passed
@rnestler
Copy link
Owner

Okay, maybe not exactly how you wanted it (I can adjust if needed) but: if verbose is not set, only notify that a reboot/session reload is requited. Withe verbose flag, print the kernel version (always) and package version (if an update is required).

Lets see how it works out in practice like this.

I also added a changelog entry for 1.7.2 with this PR.

I adapted it to be for "Unreleased"

Thanks for being kind, its my first PR!

You're welcome! Thanks for contributing 🙂

@rnestler rnestler mentioned this pull request Aug 27, 2024
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.

2 participants