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

Add black formatting in tox + rev ignore in git blame #244

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

FernandesMF
Copy link
Contributor

@FernandesMF FernandesMF commented Sep 18, 2023

As part of our effort to standardize our project repos, we are adding black formatting to freshmaker in tox.

This commit:

  • adds a step with black --check in tox, to verify the code is well formatted
  • adds a .git-blame-ignore-revs file to keep track of large reformatting commits that should be ignored by git blame
  • adds instructions on how to use .git-blame-ignore-revs automatically in a local environment
  • add E203 to flake8 ignores, to prevent conflicts with black's style

For more information, please see the following links:
https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html
https://akrabat.com/ignoring-revisions-with-git-blame/
https://stackoverflow.com/questions/34957237/can-i-configure-git-blame-to-always-ignore-certain-commits-want-to-fix-git-blam/57129540#57129540
https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

JIRA: CWFHEALTH-2279

"Testing" remarks

I checked that the CI is runnign without errors and that the new black step is being called:
https://github.com/redhat-exd-rebuilds/freshmaker/actions/runs/6252521416/job/16975967623?pr=244#step:4:1929

I also checked that the blame view in github is ignoring the large reformatting commit.

@FernandesMF FernandesMF changed the title add black step to tox Add black formatting in tox + rev ignore in git blame Sep 18, 2023
@ElenaKarolinaSemanova
Copy link
Contributor

LGTM 👍

@FernandesMF FernandesMF self-assigned this Sep 20, 2023
@FernandesMF FernandesMF marked this pull request as ready for review September 20, 2023 20:03
Copy link
Contributor

@qixiang qixiang left a comment

Choose a reason for hiding this comment

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

LGTM, good job.

iiuc, the first commit id in your PR should be preserved after merge, so the .git-blame-ignore-revs should work, but if the first commit id (c3b39ea3d4488c62d4a39c6be5b7ca5fdf6cda84) will be changed, you can open another PR to update it.

(main) A - B - - -D'
             \    |
              C - D

After merge, commit id of D will be changed, but commit id of C should be preserved.

As part of our effort to standardize our project repos, we are adding
black formatting to freshmaker in tox.

This commit:
    - adds a step with `black --check` in tox, to verify the code is
      well formatted
    - adds a `.git-blame-ignore-revs` file to keep track of large
      reformatting commits that should be ignored by `git blame`
    - adds instructions on how to use `.git-blame-ignore-revs`
      automatically in a local environment
    - add E203 to flake8 ignores, to prevent conflicts with black's style

For more information, please see the following links:
https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html
https://akrabat.com/ignoring-revisions-with-git-blame/
https://stackoverflow.com/questions/34957237/can-i-configure-git-blame-to-always-ignore-certain-commits-want-to-fix-git-blam/57129540#57129540
https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

JIRA: CWFHEALTH-2279
@FernandesMF FernandesMF merged commit e98da0a into redhat-exd-rebuilds:main Sep 21, 2023
@FernandesMF FernandesMF deleted the add-black-to-tox branch November 29, 2023 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants