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

Rethink issue labels and convential commit types #7

Closed
redeboer opened this issue Feb 25, 2022 · 4 comments · Fixed by ComPWA/policy#81
Closed

Rethink issue labels and convential commit types #7

redeboer opened this issue Feb 25, 2022 · 4 comments · Fixed by ComPWA/policy#81
Assignees
Labels
🖱️ DX Improvements to the Developer Experience ❔ Question Discuss this matter in the team

Comments

@redeboer
Copy link
Member

redeboer commented Feb 25, 2022

See Commit conventions and ComPWA repository labels.

  • There is a mismatch between the headers of the release drafter.
  • Commits that introduce interface changes should become easier to identify. Conventional commit types refactor and chore are too ambiguous.
  • Conventional commit type ci is collecting many meanings since 'local' CI, GitHub Actions, and deployment have become more synchronised (through tox).
  • The DX and Maintenance labels are also ambiguous.

Need at least labels for:

  • Interface changes: scripts that use this library may need to be modified.
  • Behaviour changes: same interface, but can output can be affected. Example: refactor: block ComplexSqrt from evaluating ampform#270.
  • New features or enhancements. It might be an idea to distinguish between additions to the interface (new features) and performance improvements (enhancement).
  • Maintenance: refactoring that does not affect behaviour or interface.
  • Improvements to the Developer Experience: some new features that make it easier to develop the framework. These include local improvements as well as improvements to CI and CD.
  • Documentation: changes that apply purely to the docs folder, to docstrings in the source code, or to any other explanatory files.
@redeboer redeboer added the 🖱️ DX Improvements to the Developer Experience label Feb 25, 2022
@redeboer redeboer self-assigned this Mar 8, 2022
@redeboer redeboer transferred this issue from ComPWA/policy Apr 25, 2022
@redeboer redeboer added the ❔ Question Discuss this matter in the team label Jul 27, 2022
@redeboer
Copy link
Member Author

redeboer commented Aug 9, 2022

Perhaps worth to consider the conventions listed here:
https://github-activity.readthedocs.io/en/stable/#splitting-prs-by-tags-and-prefixes

@redeboer
Copy link
Member Author

redeboer commented Aug 9, 2022

Settled for the following:

Commit type Repository label Description
FEAT #C2E0C6 ✨ Feature New features added
ENH #C2E0C6 ⚙️ Enhancement Improvements and optimizations of existing features
FIX #e11d21 🐛 Bug Bug has been fixed
BREAK #F9D0C4 ⚠️ Interface Breaking changes to the API
BEHAVIOR #F9D0C4 ❗ Behavior Changes that may affect the framework output
DOC #bfd4f2 📝 Docs Improvements or additions to documentation
MAINT #FFCD8F 🔨 Maintenance Maintenance and upkeep improvements
DX #FEF2C0 🖱️ DX Improvements to the Developer Experience

So an example of a commit would be:

BEHAVIOR: add non-negative assumptions to mass symbols

which would get this PR label:
image

@spflueger, do you think these cover our PR/commit types?

@spflueger
Copy link
Member

spflueger commented Aug 9, 2022

Angular conventional commit messages have "breaking change" as an additonal marker independent of the commit type https://www.conventionalcommits.org/en/v1.0.0/#summary . That seems more flexible to me. Im not aware it this can be combined nicely with the release drafter.

@redeboer
Copy link
Member Author

redeboer commented Aug 9, 2022

We used to follow these conventional commit types and allowed appending ! (which was done somewhat arbitrarily). I would say this now becomes BREAK.

The major change in the suggested scheme (#7 (comment)) is that the types of 'breaking changes' are clearer. BREAK is a change to the API that might break existing scripts and therefore require a bump in the minor version. (More mature libraries do this through slow, progressive deprecations, but we are still in 0.x.x releases). BEHAVIOR is less severe, but still needs to be clearly signaled to the release notes. It's also important because we provide physics libraries, where sometimes a certain formalism tweak is introduced that may affect physics results, but is not an API change. This was missing so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖱️ DX Improvements to the Developer Experience ❔ Question Discuss this matter in the team
Projects
Status: Upcoming
Development

Successfully merging a pull request may close this issue.

2 participants