This is an archive of the discontinued LLVM Phabricator instance.

[swig] Start of pylint on python build scripts.
ClosedPublic

Authored by brucem on Nov 4 2015, 11:42 PM.

Details

Summary

This does a broad first pass on cleaning up a lot of the noise when
using pylint on these scripts. It mostly addresses issues of:

  • Mixed tabs and spaces.
  • Trailing whitespace.
  • Semicolons where they aren't needed.
  • Incorrect whitespace around () and [].
  • Superfluous parentheses.

There will be subsequent patches with further changes that build
upon these.

Diff Detail

Repository
rL LLVM

Event Timeline

brucem updated this revision to Diff 39326.Nov 4 2015, 11:42 PM
brucem retitled this revision from to [swig] Start of pylint on python build scripts..
brucem updated this object.
brucem added reviewers: zturner, domipheus.
brucem added a subscriber: lldb-commits.
zturner edited edge metadata.Nov 5 2015, 10:05 AM
zturner added a subscriber: zturner.

Would it be possible for you to break this up into patches that only run 1
fixer at a time? When I look at the diff, basically every line has a
change, so it's hard for me to verify that no actual substance changed, and
that this is all just style issues.

Even if it were, in some of the files, almost every line would change anyway due to semicolons and then due to incorrect whitespace.

Seems ok I suppose, I can't find any functional changes from eyeballing it aside from the import changes. I'm still confused why utilsDebug.py even worked before

scripts/utilsDebug.py
14 ↗(On Diff #39326)

What's causing modules to be added / removed from the diffed version? Is this some kind of "include what you use" type of thing that detects if you have unused modules?

Speaking of which, this module got *added* not removed. How was it even working before?

brucem added inline comments.Nov 5 2015, 10:19 AM
scripts/Python/buildSwigPython.py
293 ↗(On Diff #39326)

I shouldn't have chopped an 's' from here.

655 ↗(On Diff #39326)

This was removed as it was overwriting the value of the global instead of just appending to it below.

scripts/utilsDebug.py
14 ↗(On Diff #39326)

There was a warning at the usage of sys that it wasn't defined.

It is only used in one place in this file and that's an error path that probably never ever triggers.

zturner accepted this revision.Nov 5 2015, 10:36 AM
zturner edited edge metadata.
This revision is now accepted and ready to land.Nov 5 2015, 10:36 AM
This revision was automatically updated to reflect the committed changes.