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

WIP: Customizing code convention #56

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oesteban
Copy link
Contributor

@oesteban oesteban commented Aug 22, 2023

Requires: #55 .

@pawamoy please have a look at the usage.md file changes w.r.t. the #55 feature branch - does it sound like a reasonable plan? Quick link for the diff: oesteban/git-changelog@feat/config-files...oesteban:git-changelog:feat/modify-commit-convention

@oesteban oesteban marked this pull request as draft August 22, 2023 07:30
@oesteban oesteban force-pushed the feat/modify-commit-convention branch from 31d33fa to f51cd47 Compare August 22, 2023 08:42
@pawamoy
Copy link
Owner

pawamoy commented Aug 22, 2023

You can rebase on main, I've merged #55 🙂

@pawamoy
Copy link
Owner

pawamoy commented Aug 22, 2023

Sounds good, I have two questions/remarks though:

  • I'm not completely sold on the idea of populating sections automatically when the custom convention is selected and the sections setting is unset. I'll have to see more examples of use to be convinced. Maybe this shows a flaw where the sections setting should actually be attached to a particular convention, allowing to define sections for multiple conventions, and switch from one to another on the CLI or by changing the convention setting.
  • Why does it say that only the first type of commit with the same section title will be used? I don't think that's how it works right now? You can both have test and tests commits, and both kinds will be picked up for the Tests section.

@pawamoy
Copy link
Owner

pawamoy commented Aug 22, 2023

Also, such an implemention would mean that convention are not entirely custom, but rather based on the conventional (or angular) convention, right? Therefore all custom conventions are based on the type(scope): Summary regex for the commit message, and on the same breaking changes regex for the body.

That would be OK to me. We'd just need to explain that in the docs. And we can see later if users ask for more customization.

Fortunately, (provider) references and trailers can be parsed independently 🙂 But there might be a few things to update for references since they will affect the remote URLs 🤔 Anyway, I'm digressing.

@oesteban oesteban force-pushed the feat/modify-commit-convention branch from f51cd47 to b25211e Compare August 22, 2023 09:15
@oesteban
Copy link
Contributor Author

  • I'm not completely sold on the idea of populating sections automatically when the custom convention is selected and the sections setting is unset. I'll have to see more examples of use to be convinced. Maybe this shows a flaw where the sections setting should actually be attached to a particular convention, allowing to define sections for multiple conventions, and switch from one to another on the CLI or by changing the convention setting.

I'm not 100% sure the proposal is a good idea - let's see how it looks as a draft and move from there.

  • Why does it say that only the first type of commit with the same section title will be used? I don't think that's how it works right now? You can both have test and tests commits, and both kinds will be picked up for the Tests section.

I think the problem here is in the documentation I've written - it's not clear, but this intends to actually implement the behavior that is already in git-changelog.

Also, such an implemention would mean that convention are not entirely custom, but rather based on the conventional (or angular) convention, right? Therefore all custom conventions are based on the type(scope): Summary regex for the commit message, and on the same breaking changes regex for the body.

I really can't anticipate side effects, but as I was writing the draft, there's no gain between creating a new convention or altering an existing one. The regex seems a good reason to modify a convention rather than creating a custom one.

This means that maybe we will need more granular editing of the convention, for example:

  • editing including the regex,
  • allowing editing the commit type mapping (rather than fully rewriting it) -- I couldn't come up with a straightforward way of removing commit types (adding is easy).

@oesteban
Copy link
Contributor Author

Okay, perhaps e85bcc3 makes more sense.

I'm also considering making the configuration more granular, something like:

[tool.git-changelog]
convention = 'myconvention'
sections = ['fix', 'enh', 'doc', 'maint', 'sty']

[tool.git-changelog.conventions.myconvention]
commit-type = [
  ['fix', 'Bug fixes'],
  ['enh', 'New features and enhancements'],
  ['feat', 'New features and enhancements'],
  ['doc', 'Documentation'],
  ['docs', 'Documentation'],
  ['maint', 'Maintenance and continuous integration'],
  ['chore', 'Maintenance and continuous integration'],
  ['ci', 'Maintenance and continuous integration'],
  ['sty', 'Code style and comments'],
  ['style', 'Code style and comments']
]
regex = "..."

Using a list of pairs could workaround the limitations of toml with nested dictionaries.

@pawamoy
Copy link
Owner

pawamoy commented Aug 22, 2023

editing including the regex,

That is exactly the territory I dare not enter again 😁 IMO if you allow users to edit the regex, they will want to do things that are not supported by the Python code. Then you'll maybe want to code something generic that supports any regex, and from experience that will not go well. Furthermore, we already see from the single version-regex setting that writing Python regex in TOML is not super fun.

I'd like to state again that I was not in favor of creating custom convention through TOML, but that I'd still review a PR bringing such a feature. An implementation allowing any regular expression in the config file, to be used magically in Python, is not something I'd want to maintain in the long term.

So, to rephrase my previous messages, which might not have been super clear, I'm OK with limiting ourselves to just allowing redefinition of sections (section title and commit "type" in terms of angular/conventional commits), and nothing more.

You are free to elaborate on what you would like to support in git-changelog of course, and change my mind 😄
I guess your use-case is exactly the one described in the changes here? Modifying the commit types and section titles?

@oesteban
Copy link
Contributor Author

Sure, I don't think allowing changing the regex is a good idea. Okay, let's go with the dictionary that limits edits to just the commit type and title.

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.

2 participants