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