Make printer width configurable #1247
Conversation
There was a problem hiding this comment.
Thanks for the PR @kasanchez! This looks really great!
I think there were some unit test failures around pep8 (our coding style guide). I just dropped some comments in indicating where a few more newlines are required.
Overall though, I think this is super slick and will be a great improvement to dbt :). I'm really impressed with the quality of this code and I'm excited to merge it when the tests pass!
Lmk if you need a pointer for where to add tests, happy to help
| @@ -130,6 +130,8 @@ def initialize_config_values(parsed): | |||
| if cfg.use_colors: | |||
| dbt.ui.printer.use_colors() | |||
|
|
|||
| if cfg.printer_width: | |||
| dbt.ui.printer.printer_width(cfg.printer_width) | |||
There was a problem hiding this comment.
pep8: can you add an extra newline here (there should be two lines between functions)
|
|
||
| def use_colors(): | ||
| global USE_COLORS | ||
| USE_COLORS = True | ||
|
|
||
| def printer_width(printer_width): |
There was a problem hiding this comment.
pep8: can you add an extra newline above and below this function?
|
hey @drewbanin thanks for the feedback! I could use a pointer on where to add some tests. I'll update my pr to address the pep8 comments above as well |
|
@kasanchez you can make some tiny additions to this function: https://github.com/fishtown-analytics/dbt/blob/dev/stephen-girard/test/unit/test_config.py#L229 I'd try editing the |
|
@kasanchez what's the happs with this PR? I think we'll want to rebase it against the The code here all LGTM, but looks like something funky happened with the integration tests :/ |
|
@kasanchez I merged this over in #1459 with a tiny additional test. I did some git magic to make sure you get credit for the commit -- thanks for your contribution to dbt!! 🎉 |
closes #1026
Tests to come.
I'm open to feedback!