-
Notifications
You must be signed in to change notification settings - Fork 31
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 cli options to output lookml #89
base: master
Are you sure you want to change the base?
Conversation
json_string = json.dumps(result, indent=2) | ||
print(json_string) | ||
|
||
finally: |
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.
FileType objects dont work with context managers, this makes sure the file is closed even if they cannot be parsed.
517dd00
to
0006a83
Compare
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.
Hey @remisalmon thank you for this PR and I'm sorry it's taken so long for me to review it. My only question is with the --write
arg. Can you explain to me what you would use this for?
lkml/__init__.py
Outdated
group.add_argument( | ||
"-w", | ||
"--write", | ||
action="store_true", | ||
default=False, | ||
help="parse and write back to file", | ||
) |
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.
What's the use case for parsing from a file and writing back to the same file?
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.
The lkml parser/dumper does a great job at cleaning up lkml files (separating measures from dimensions, correcting the indentation, etc.), the option to write it back to file would allow to use lkml
as a auto-formatter in text editors, pre-commits, CI checks, etc. (like running black
on .py files).
fafcb92
to
0976e5a
Compare
0976e5a
to
f78206a
Compare
Hi @joshtemple I have removed that option, happy to add it back if you feel it belongs here. I replied just above in #89 (comment). I also added some cli tests to make sure the output (json or lookml) can be parsed back. |
This should close #88.