-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce pre-commit with pre-commit suggestions #4972
Conversation
40e2c68
to
81f2d2e
Compare
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
81f2d2e
to
0e54f88
Compare
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
I'm seeing quite a lot of code changes from Black, might need to play with the formatter to get it to replicate the current setup. |
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
for more information, see https://pre-commit.ci
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Since we were using a really old version of Black, I was running into this: https://stackoverflow.com/questions/71673404/importerror-cannot-import-name-unicodefun-from-click All suggestions say to upgrade Black, upgrading black also updated the formatting for many files. I think on some quick inspections the suggestions are fine (imo) but @ericharper @titu1994 let me know if it's reasonable. I think eventually we would drop the style check from jenkins to reduce time. |
Going to wait till 1.12 is released, so that we can pull this in without causing cherry-picking issues. |
Done in #5027 |
What does this PR do?
Currently style checking is done within the
setup.py
and enforced via jenkins.This is problematic for users who do not have jenkins access and builds fail due to simple style fixes. By using pre-commit CI, we can automate style fixes in PRs meaning less work for users (and have a dedicate CI job for this).
As a side effect of this, due to the old version of black running I had to upgrade the Black version from the pre-release (19) to a stable version (22.*). Due to this there has been some additional style changes. A few summarized below:
i.e
Changelog
Before your PR is "Ready for review"
Pre checks:
PR Type: