Skip to content

[Python 2->3] Run futurize as part of butler.py lint#760

Merged
mbarbella-chromium merged 8 commits into
masterfrom
futurize-lint
Aug 1, 2019
Merged

[Python 2->3] Run futurize as part of butler.py lint#760
mbarbella-chromium merged 8 commits into
masterfrom
futurize-lint

Conversation

@mbarbella-chromium

Copy link
Copy Markdown
Contributor

PTAL when you have a chance.

@mbarbella-chromium

Copy link
Copy Markdown
Contributor Author

/gcbrun

@inferno-chromium inferno-chromium left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot!

Comment thread src/local/butler/lint.py Outdated
Comment thread src/python/platforms/fuchsia/util/device.py Outdated
Comment thread src/local/butler/lint.py Outdated
futurize_output = _execute_command_and_track_error(
'futurize -0 -x libfuturize.fixes.fix_unicode_keep_u '
'-x libfuturize.fixes.fix_future_builtins -x lib2to3.fixes.fix_dict '
'-x lib2to3.fixes.fix_ws_comma -x lib2to3.fixes.fix_idioms ' +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why fix_ws_comma ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

idioms and ws_comma are disabled by default and not explicitly disabling them makes the tool print an extra log line for each. Just did it to save some spam here. I'll leave a comment explaining that when addressing the first comment.

@mbarbella-chromium

Copy link
Copy Markdown
Contributor Author

/gcbrun

@mbarbella-chromium mbarbella-chromium merged commit 0c8bf1c into master Aug 1, 2019
@mbarbella-chromium mbarbella-chromium deleted the futurize-lint branch August 1, 2019 20:21

@oliverchang oliverchang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, thanks! LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes CLA signed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants