feat: Customizing code convention

[sandzhaj]

Issue: #59
Resolves: #59

[sandzhaj]

Given that the CommitConvention classes utilize class attributes (rather than instance attributes), the process becomes slightly intricate. When updating self.TYPES, it's crucial for the regex to be updated as well. Consequently, we're presented with two options: either use a property or relocate it under __init__. I'm not sure which choice is superior.

[pawamoy]

Thanks for the PR 🙂

• I prefer the TOML hierarchy used in WIP: Customizing code convention #56, as it allows to customize several conventions (maybe at some point we'll want to implement a fallback mechanism when a commit cannot be parsed):

  [tool.git-changelog.conventions.conventional]
  build = "Build"
  chore = "Chore"
  ci = "Continuous Integration"
  deps = "Dependencies"
  doc = "Documentation"
  feat = "Features"
  n = "Notes"

• I'm not sure why sections would become mandatory if we simply customized the titles of sections (translations) without adding or removing any type.

• This PR starts showing some design flaws in the original code, namely class variables. I'd much prefer we refactor to initialize types, regular expressions, etc., in a ___init__ method (as you pointed out in one of your comments), defaulting to the class variable values:

  # rough example
  class CommitConvention(ABC):
      TYPES: ClassVar[dict[str, str]]
      MINOR_TYPES: ClassVar[list[str]]
  
      def __init__(self, types = None, minor_types = None):
          self.types = types or self.TYPES
          self.minor_types = minor_types or self.MINOR_TYPES

  This way we can simply instantiate a convention class with the user configured types, etc., and avoid classmethods that modify class attributes which, as we can see in tests, are not fun to deal with, and will be even less fun for a programmatic use of the project (as a library).

  This will also let us get rid of the properties for regex since we'll be able to initialize them once and for all, providing better performance at the same time.

• Finally, this PR moves some of the "conventional" logic (minor types) into the base convention class, something I would like to avoid since all conventions do not necessarily know anything about commit types. I understand the benefit of factorization though, so instead we can add an intermediate abstraction layer between the base class and the subclasses that implement "conventional" conventions: Base -> ConventionalBase -> ConventionalConvention/AngularConvention.

I understand this represent consequent work for this PR, but it's important we take the time to refactor when we have a chance to, otherwise this project will be less and less maintainable. I can definitely help with these refactors, if you're OK with me pushing to your branch 🙂

[sandzhaj]

I prefer the TOML hierarchy used in #56, as it allows to customize several conventions (maybe at some point we'll want to implement a fallback mechanism when a commit cannot be parsed)

We can't run several conventions at the same time. Everytime you run the script it takes only one convention. Even if you want to “play”, you can be satisfied with commented block in pyproject.toml. I can't imagine any practice use, where commented blocks won't be enough.

I'm not sure why sections would become mandatory if we simply customized the titles of sections (translations) without adding or removing any type.

English is not my native language, so sometimes I Could talk unclearly. When I say

“You can overwrite default types with the rewrite-convention parameter.”

It also means, that you free to add or remove any type.

example:

[tool.git-changelog.rewrite-convention]
bu = "Burger"
co = "Cola"

Now you have only two types. With no feat etc. You have to specify sections

[sandzhaj]

PR moves some of the "conventional" logic (minor types) into the base convention class, something I would like to avoid since all conventions do not necessarily know anything about commit types. I understand the benefit of factorization though, so instead we can add an intermediate abstraction layer between the base class and the subclasses that implement "conventional" conventions: Base -> ConventionalBase -> ConventionalConvention/AngularConvention.

Actually I move to the ConventionalBase only function definition. MINOR_TYPES values are still uniq and defined inside every convention. It is an example of refactoring, named “Extract Method“ - https://refactoring.guru/extract-method

This PR starts showing some design flaws in the original code, namely class variables. I'd much prefer we refactor to initialize types, regular expressions, etc., in a _init method (as you pointed out in one of your comments), defaulting to the class variable values

Agree. That will be the best way, I think

if you're OK with me pushing to your branch 🙂

sure, no problem. Thanks!

[pawamoy]

We can't run several conventions at the same time.

Yes, that's why I mentioned the possible future "fallback" feature. But I'm probably overthinking. You are right that we should keep things simple. I'm not a fan of rewrite-convention though and will try to come up with a name I prefer.

It also means, that you free to add or remove any type.

That is what I understood yes. Specified types are not merged with default types. That's good. However, in the code I see:

if rewrite_convention and not sections:
    raise ValueError(...)

This code does not try to verify if new types were added, or if types were removed. Maybe the user is fine with the default selected sections and their ordering, and just want to translate their titles. In that case, they wouldn't define sections and the code shouldn't raise an exception. I think we can do better here, either by verifying more things, implementing a default behavior for sections, or by catching errors and displaying helpful error messages. I'd vote for the second option, as hinted in #56, where undefined sections would be populated with the list of existing types.