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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

move black & flake8 to precommit #739

Merged
merged 3 commits into from
Nov 13, 2023
Merged

move black & flake8 to precommit #739

merged 3 commits into from
Nov 13, 2023

Conversation

Borda
Copy link
Member

@Borda Borda commented Nov 13, 2023

What does this PR do?

It shall simplify maintenance since we already have the preco-commit configuration with installing this precommit bot (free for public repos); it will eventually apply fixes for incorrect formatting so the user doe/contributor does not need to think about it... I have been using it on several other projects and it is very helpful without any overhead... 馃惏

Also, after adding the bot requited check will need to be updated

@Borda Borda added enhancement needs:discussion additional discussion is requested labels Nov 13, 2023
@Borda Borda requested review from MSeal and willingc November 13, 2023 05:20
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #739 (0963dab) into main (8f16a9f) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #739   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files          17       17           
  Lines        1633     1633           
=======================================
  Hits         1494     1494           
  Misses        139      139           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 8f16a9f...0963dab. Read the comment docs.

Copy link
Member

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @Borda for improving this. One small change is requested and then I'm happy to approve.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

Thanks for making the requested change! I'll let @willingc do the merge in case she wants anything else adjusted :)

Copy link
Member

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and the improvements. Happy to merge. 馃憤馃徏

@willingc willingc merged commit 9d8b429 into main Nov 13, 2023
12 checks passed
@willingc willingc deleted the precommit/black-flake8 branch November 13, 2023 23:50
@Borda
Copy link
Member Author

Borda commented Nov 14, 2023

Thanks for the fix and the improvements. Happy to merge. 馃憤馃徏

Could you pls install the precommit bot and set it as required check so we preserve the code quality bar... 馃惏

Probably it need to be someone with org admin 馃Ι

@willingc
Copy link
Member

@Borda I think that I have precommit setup now. Please check to see if it looks correct to you on #738

@Borda
Copy link
Member Author

Borda commented Nov 14, 2023

@Borda I think that I have precommit setup now. Please check to see if it looks correct to you on #738

I don't see it there, except the config file the bot need to be installed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs:discussion additional discussion is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants