Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20169 )

Change subject: IMPALA-11980 (part 1): Put all thrift-generated python code 
into the impala_thrift_gen package
......................................................................


Patch Set 12:

> > Patch Set 12: Code-Review+1
 > >
 > > Might be good idea to sort the import order. But don't mind if
 > not, considering the change size.
 >
 > isort seems useful, but might be too aggressive for us.
 > https://pycqa.github.io/isort/docs/configuration/profiles.html
 >
 > This works for me:
 >
 > git show HEAD --name-only | grep 'tests.*py' \
 > | xargs -I {} isort --profile attrs --line-width 90 {}

Yeah, isort seems like a good idea if we can come up with an appropriate 
configuration. I've been playing around with that too. My command right now 
looks like:
isort -i 4 -m 3 --dt -l 90 --os --fass --fss -p tests -p impala_thrift_gen -o 
builtins

The sorting part is great, but it also applies a style to multiline imports. 
We're very inconsistent about that, so it can result in a variety of line 
changes. I'm basically fine with that, as long as other people are ok with it.


--
To view, visit http://gerrit.cloudera.org:8080/20169
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2927f22c7257aa38a78084efe5bd76d566493c0
Gerrit-Change-Number: 20169
Gerrit-PatchSet: 12
Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Apr 2025 00:44:47 +0000
Gerrit-HasComments: No

Reply via email to