Limit argument range for --verbose, make default log level more explicit. #93

floli · 2018-09-14T06:48:16Z

floli commented

2018-09-14 06:48:16 +00:00

See #92

witten reviewed 2018-09-15 22:37:49 +00:00

witten left a comment

Looks good. Thanks for the fix!

borgmatic/commands/borgmatic.py Outdated

						
				@@ -92,6 +92,8 @@ def parse_arguments(*arguments):

				    parser.add_argument(

				        '-v', '--verbosity',

				        type=int,

				        choices=range(0,3),

witten commented

2018-09-15 22:34:56 +00:00

At first I forgot that the end of range() is non-inclusive, and thought you were adding a new super-debug log level. But then I figured it out. :)

At first I forgot that the end of `range()` is non-inclusive, and thought you were adding a new super-debug log level. But then I figured it out. :)

borgmatic/verbosity.py Outdated

						
				@@ -5,1 +4,3 @@

				VERBOSITY_LOTS = 2

				VERBOSITY_WARNING = 0

				VERBOSITY_SOME    = 1

				VERBOSITY_LOTS    = 2

witten commented

2018-09-15 22:33:06 +00:00

Minor code style thing: I generally try to avoid manually lining up operators like this across lines with spacing. So I think I'd prefer this alterative if you don't mind:

VERBOSITY_WARNING = 0
VERBOSITY_SOME = 1
VERBOSITY_LOTS = 2

Minor code style thing: I generally try to avoid manually lining up operators like this across lines with spacing. So I think I'd prefer this alterative if you don't mind: ``` VERBOSITY_WARNING = 0 VERBOSITY_SOME = 1 VERBOSITY_LOTS = 2 ```

witten commented

2018-09-15 22:36:18 +00:00

I'm inspired by PEP-8 on this topic, by the way: https://www.python.org/dev/peps/pep-0008/#pet-peeves (scroll down to the last example in that section).

I'm inspired by PEP-8 on this topic, by the way: [https://www.python.org/dev/peps/pep-0008/#pet-peeves](https://www.python.org/dev/peps/pep-0008/#pet-peeves) (scroll down to the last example in that section).

floli commented

2018-09-17 06:58:48 +00:00

Ok, fixed the indention. I changed my commit and did a force push, so don't be surprised.

witten closed this pull request

2018-09-18 05:31:28 +00:00

witten commented

2018-09-18 05:31:37 +00:00

Cool, thank you!

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#93