-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Print level feature #33
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ def __init__( | |
path=None, | ||
file_types=None, | ||
white_listed_files=None, | ||
print_all=True, | ||
print_level=None, | ||
include_patterns=None, | ||
): | ||
""" | ||
|
@@ -34,7 +34,7 @@ def __init__( | |
|
||
Args: | ||
- path (str) : full path to the root folder to check. If not defined, no file_paths are parsed | ||
- print_all (str) : control var for whether to print all checked file names or only the ones with urls. | ||
- print_level (str) : control the printing level. | ||
- white_listed_files (list) : list of white-listed files and patterns for flies. | ||
- include_patterns (list) : list of files and patterns to check. | ||
""" | ||
|
@@ -45,7 +45,7 @@ def __init__( | |
# Save run parameters | ||
self.white_listed_files = white_listed_files or [] | ||
self.include_patterns = include_patterns or [] | ||
self.print_all = print_all | ||
self.print_level = print_level | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all the places that print_level is set from a client, we have to assume that a user might set it to None, thinking this is default or "unset." So instead of just blindly setting it like this, let's do something like:
That way if it's set to an empty string or None, it will not break the client. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But since we favor verbosity, the default should be: self.print_level = print_level or "all" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense. I will make the change. |
||
self.path = path | ||
self.file_types = file_types or [".py", ".md"] | ||
self.file_paths = [] | ||
|
@@ -155,7 +155,7 @@ def run( | |
file_name=file_name, | ||
white_listed_patterns=white_listed_patterns, | ||
white_listed_urls=white_listed_urls, | ||
print_all=self.print_all, | ||
print_level=self.print_level, | ||
) | ||
|
||
# Check the urls | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is files-with-urls-only honored? I don't see it in the printing function, only the other four.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so
files-with-urls-only
is not explicitly specified in the code, because I did not need to. Covering the other cases was sufficient. So -as you already noticed- you barely see the variable in the code, but if used, it works as expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn’t make any sense. Please provide an explicit example. If an option isn’t used I don’t understand why it exists.