ansys / pymapdl

Pythonic interface to MAPDL
https://mapdl.docs.pyansys.com
MIT License
419 stars 116 forks source link

feat: check commit message type before committing #3187

Closed germa89 closed 1 week ago

germa89 commented 1 week ago

Description

Use pre-commit hooks to check commit messages before commiting.

Issue linked

N/A

Checklist

ansys-reviewer-bot[bot] commented 1 week ago

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.47%. Comparing base (7c1eb1e) to head (d4b0b0b). Report is 13 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3187 +/- ## ========================================== - Coverage 86.63% 84.47% -2.17% ========================================== Files 52 52 Lines 9550 9596 +46 ========================================== - Hits 8274 8106 -168 - Misses 1276 1490 +214 ```
clatapie commented 1 week ago

As the action only checks for the PR name, this will make the pre-commit and therefore the code-style action stricter than it is at present. Also, some automatic commit names such as Merge branch 'main' into ..... or Adding changelog entry: ..... will failed.

Edit: as there is no code-style in the CI/CD, it will not fail because of it.

germa89 commented 1 week ago

As the action only checks for the PR name, this will make the pre-commit and therefore the code-style action stricter than it is at present. Also, some automatic commit names such as Merge branch 'main' into ..... or Adding changelog entry: ..... will failed.

Edit: as there is no code-style in the CI/CD, it will not fail because of it.

Good catch @clatapie

OK. I think the pyansys-ci-bot should have a proper commit message... at the end of the day, we are trying to enforce this. @klmcadams can you give us an option in the changelog to add a prefix to the commit message? Pinging @RobPasMue for feedback.

Regarding the Merge to main... I guess there is no way... we merge to main from github website... and also we use VSCode. I will see if we can have a custom commit message.

Although we do not have code-style action, we do have pre-commit as part of the CICD. We are just missing the docker linter. We could add it though.

Let's merge this if there is no failure on CICD.

germa89 commented 1 week ago

Merge to main... could be a big headache. Let's park this for the moment.

RobPasMue commented 1 week ago

@klmcadams can you give us an option in the changelog to add a prefix to the commit message? Pinging @RobPasMue for feedback.

Agreed - in fact we should standardize it to "chore: ..." IMO

RobPasMue commented 1 week ago

Merge to main... could be a big headache. Let's park this for the moment.

Yeah... you are not going to come around that one easily 😄

germa89 commented 1 week ago

Merge to main... could be a big headache. Let's park this for the moment.

Yeah... you are not going to come around that one easily 😄

History of my life man... xDDD

germa89 commented 1 week ago

I'm closing this PR without merging. Fixing it is more trouble than getting used to the new convention.