Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Use absl.app in print_log_list.py and improve flag checks #1453

Merged
merged 3 commits into from Sep 10, 2018

Conversation

RJPercival
Copy link
Contributor

Using absl.app causes flags to be handled automatically and gives us nice things like app.UsageError. I've added extra checks for unspecified flags, since otherwise the error messages caused by these flags being unset were quite obtuse.

I've removed the default value for --log_list_schema since there is no schema file found at "data/log_list_schema.json".

pphaneuf
pphaneuf previously approved these changes Sep 7, 2018
Copy link
Contributor

@pphaneuf pphaneuf left a comment

Choose a reason for hiding this comment

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

Just one minor tweak, thanks!

python/utilities/log_list/print_log_list.py Outdated Show resolved Hide resolved
Using absl.app causes flags to be handled automatically and gives us
nice things like app.UsageError. I've added extra checks for unspecified
flags, since otherwise the error messages caused by these flags being
unset were quite obtuse.

I've removed the default value for --log_list_schema since there is no
schema file found at "data/log_list_schema.json".

# Conflicts:
#	python/utilities/log_list/print_log_list.py
def run():

def main(argv):
del argv # Unused; all arguments provided via flags.
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually, what we do with unused flags like here is name them unused_argv (which shuts up the linter).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My linter sadly does complain if I name it unused_argv. If I prefix it with an underscore, it's happy though.

@RJPercival RJPercival merged commit 1e90c61 into google:master Sep 10, 2018
@RJPercival RJPercival deleted the log_list_app branch September 10, 2018 14:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants