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

badfiles: displaying results only for corrupted files by default #2434

Merged
merged 2 commits into from
Feb 10, 2017

Conversation

karpinski
Copy link
Contributor

Per request from #1654.

As a continuation to the discussion on the change: I think it would make sense to have a separate option for displaying all checker results (including the positive ones) with only bad files being displayed by default. The reason for having that, instead of logging the results for the uncorrupted files at the debug level, is that using this option the user would be able to see the results for all of the files without seeing a bunch of other debug messages, making the output less messy. The option could be named --all or --show-all, in order to be distinguished from the global --verbose.

This is just a suggestion, though. Let me know if you'd rather have the positive checker results be logged on the debug level, instead of introducing this new option.

@jackwilsdon
Copy link
Member

jackwilsdon commented Feb 9, 2017

I like this idea, as badfiles creates quite a lot of unnecessary noise at the moment. Travis doesn't like this change as it looks like there's some left over whitespace:

beetsplug/badfiles.py:123:33: W291 trailing whitespace

It might be worth documenting this in the changelog too, but it's up to @sampsyo in the end. 😁

@sampsyo
Copy link
Member

sampsyo commented Feb 9, 2017

Sure; this looks good! I think just putting the "ok" lines behind the verbose flag will work well.

As @jackwilsdon says, a changelog entry would be great! With that and the whitespace fixed, this should be good to go.

@karpinski
Copy link
Contributor Author

Done :)

@jackwilsdon
Copy link
Member

Thank you! 🎉

@jackwilsdon jackwilsdon merged commit 8ef9f68 into beetbox:master Feb 10, 2017
@sampsyo sampsyo mentioned this pull request Apr 8, 2017
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