search

zephyrproject-rtos / ci-tools

CI Tools and Scripts (obsolete)
Apache License 2.0
9 stars 18 forks source link

check_compliance.py/what_changed.py: Remove 'sh' library dependency and fix pylint warnings #88

Closed ulfalizer closed 5 years ago

ulfalizer commented 5 years ago

Two fixes, which together get rid of all warnings for what_changed.py and get rid of the sh library.

Getting rid of the sh library was prompted by fixing the warnings, but it seems to have other nice side effects too, like Windows compatibility (for the Git part at least).

what_changed.py: Remove unused variables and imports

Fixes pylint warnings and makes the script easier to read.

Also fix two regex-related warnings:

    what_changed.py:273:0: W1401: Anomalous backslash in string: '\.'.
    String constant might be missing an r prefix.
    (anomalous-backslash-in-string)

There's still two warnings related to the 'sh' library. Those will be
fixed separately by removing it.
check_compliance.py/what_changed.py: Remove 'sh' library dependency

Having this third-party library just for the git() helper is a bit
overkill. Use the plain subprocess module instead.

Also improve error reporting, e.g. to show the command when things fail.
In check_compliance.py, the sys.exit()s will generate SystemExit
exceptions that trickle through to the top-level and get reported to
GitHub.
ulfalizer commented 5 years ago

Planning to do the same to what_changed.py, but there's a bunch of pylint stuff that needs to be fixed in it first.

zephyrbot commented 5 years ago

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

marc-hb commented 5 years ago

Having this third-party library just for the git() helper is a bit overkill. Use the plain subprocess module instead.

@carlescufi , others?, this should make this script portable to Windows.

ulfalizer commented 5 years ago

This PR now gets rid of all pylint warnings in what_changed.py too. Got messy to split it up, because CI would fail as soon as you touched what_changed.py.

ulfalizer commented 5 years ago

Maybe the git() helper could be moved into a separate .py along with other shared helpers later on. Think @marc-hb was thinking of something similar.

ulfalizer commented 5 years ago

Do you understand why username = os.environ.get('GH_USERNAME', 'zephyrbot') is not used?

Suspect it was copied over from check_compliance.py, where it's used.

Could be "will get back to this later" stuff too, but that's confusing for other devs.