Skip to content
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

Update black to the latest version incrementally #8476

Merged
merged 61 commits into from
May 10, 2024
Merged

Update black to the latest version incrementally #8476

merged 61 commits into from
May 10, 2024

Conversation

artbataev
Copy link
Collaborator

@artbataev artbataev commented Feb 21, 2024

What does this PR do?

Update black using incremental changes only for files changed in PRs.

  • the black package is updated to the latest version
  • all collections are excluded, so black . will reformat nothing
    • it is possible also to exclude collection from the list, once it is reformatted though PRs
  • there will be 2 approaches to reformat the files manually:
    • pass the file(s) explicitly (e.g., black my_file.py)
    • use a pre-commit hook
  • the automatic reformatting is done by GitHub Actions only for PRs and only for changed files

Tests (target branch is update_black):

Approach:

  • create a custom GitHub Action responsible for reformatting the code. For black use only changed files
  • custom token is used to push changes, since only custom token can trigger workflows after reformatting

Q&A About the Approach

Why do we need a custom GitHub Action if we have pre-commit.ci?

The pre-commit hook is still active and works, but pre-commit.ci reformats all the code (the pre-commit.ci team explicitly denied changing this behavior, see pre-commit-ci/issues#90), thus it is replaced with a custom GitHub Action, which does the same, but only for the changed files (in the PR)

Why do we need isort GitHub action?

Since both isort and black can change files, this should be done in one action to avoid concurrent changes. So, the GitHub Action will execute both formatters and commit changes if needed (same behavior as for pre-commit.ci).

Why do we need a custom GitHub token?

When GitHub action pushes the commit with the default secret token, it cannot trigger other workflows.
See the discussion: https://github.com/orgs/community/discussions/25702
This can only be done with custom tokens.

Collection: [CI]

Changelog

  • Add specific line by line info of high level changes in this PR.

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this 

Jenkins CI

To run Jenkins, a NeMo User with write access must comment jenkins on the PR.

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

  • Related to # (issue)

@artbataev artbataev changed the title Update to latest black incrementally Update black to the latest version incrementally Feb 23, 2024
artbataev and others added 11 commits February 23, 2024 16:50
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
artbataev and others added 9 commits February 23, 2024 16:52
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: artbataev <artbataev@users.noreply.github.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Copy link
Contributor

github-actions bot commented Mar 9, 2024

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Mar 9, 2024
Copy link
Contributor

This PR was closed because it has been inactive for 7 days since being marked as stale.

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
@github-actions github-actions bot added the ASR label Apr 22, 2024
Signed-off-by: artbataev <artbataev@users.noreply.github.com>
@github-actions github-actions bot removed the ASR label Apr 22, 2024
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
@artbataev
Copy link
Collaborator Author

Test changes from fork: #8996

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
@github-actions github-actions bot added the TTS label Apr 22, 2024
Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
@github-actions github-actions bot removed the TTS label Apr 22, 2024
@artbataev artbataev marked this pull request as ready for review April 23, 2024 12:54
@titu1994 titu1994 merged commit 1d75086 into main May 10, 2024
128 checks passed
@titu1994 titu1994 deleted the update_black branch May 10, 2024 04:05
BoxiangW pushed a commit to BoxiangW/NeMo that referenced this pull request Jun 5, 2024
* Update black version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Skip pre-commit.ci

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Allow to fix PRs

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix requirements

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Add github action for reformatting

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Update github action

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test change python file

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Use allowed github actions

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Remove installation

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix add-and-commit

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test DCO

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Clean up

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Clean up

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix isort version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix isort version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix isort version and add comments

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Trigger only on PRs

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Fix action (requires token)

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Update formatter versions

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test reformat file

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Add comments

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Check with default github token

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Use custom token

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test user

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Remove user

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Test change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Use github token

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Add token to changed files

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test changes

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Fix fetch depth

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix fetch depth

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Avoid setting repository

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Avoid setting ref

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Use pull_request_target

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Add comments

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Revert change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

---------

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: artbataev <artbataev@users.noreply.github.com>
Co-authored-by: artbataev <artbataev@users.noreply.github.com>
Signed-off-by: Boxiang Wang <boxiangw@nvidia.com>
rohitrango pushed a commit to rohitrango/NeMo that referenced this pull request Jun 25, 2024
* Update black version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Skip pre-commit.ci

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Allow to fix PRs

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix requirements

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Add github action for reformatting

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Update github action

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test change python file

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Use allowed github actions

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Remove installation

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix add-and-commit

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test DCO

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Clean up

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Clean up

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix isort version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix isort version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix isort version and add comments

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Trigger only on PRs

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Fix action (requires token)

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Update formatter versions

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test reformat file

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Add comments

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Check with default github token

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Use custom token

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test user

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Remove user

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Test change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Use github token

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Add token to changed files

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test changes

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Apply isort and black reformatting

Signed-off-by: artbataev <artbataev@users.noreply.github.com>

* Fix fetch depth

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix fetch depth

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Avoid setting repository

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Avoid setting ref

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Use pull_request_target

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Fix version

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Add comments

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Test change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

* Revert change

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>

---------

Signed-off-by: Vladimir Bataev <vbataev@nvidia.com>
Signed-off-by: artbataev <artbataev@users.noreply.github.com>
Co-authored-by: artbataev <artbataev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants