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

feat(template): allow to override the template from cli, configuration and plugins #645

Merged
merged 6 commits into from
Oct 18, 2023

Conversation

noirbizarre
Copy link
Member

@noirbizarre noirbizarre commented Dec 26, 2022

Description

This PR provides full template customization by plugins, configuration and command line.

Plugins can now contribute a template (by providing their own jinja PackageLoader as template_loader attribute) as well as more template variables with their defaults (using the template_extras attribute from `BaseCommitizen). It was my initial use case, I was monkey-patching commitizen like this

Users can now provide their own template (in the format of their choice, like .rst in #384, with extras variables too) by:

  • providing a keep_a_changelog.j2 file at the root of their project
  • setting the template config to a project root-relative path to their template
  • using the --template/-t parameter in both bump and changelog command

They also can provide or override some template variables by:

  • setting the extras config which is a dict
  • using the --extra/-e parameter (can be used as many times as there are variables) in both bump and changelog

This change is backward compatible, fully tested and documented.

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes
  • Make template overridable
  • Allow extra template parameters
  • Allow incremental with overridden templates
  • Provides examples for other syntaxes
    • ReStructuredText
    • AsciiDoc
    • Textile

Additional context

Fixes #132
Fixes #384
Fixes #433
Closes #376 (I didn't keep the tree to releases variable renaming as it is a breaking change (I can however submit this as another PR because I think the releases naming is more explicit)

@noirbizarre noirbizarre force-pushed the feature/template-override branch 2 times, most recently from 45d3ceb to 2f63807 Compare December 26, 2022 20:32
@codecov
Copy link

codecov bot commented Dec 26, 2022

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Files Coverage Δ
commitizen/__version__.py 100.00% <100.00%> (ø)
commitizen/bump.py 100.00% <100.00%> (ø)
commitizen/changelog.py 100.00% <100.00%> (+0.50%) ⬆️
commitizen/changelog_formats/asciidoc.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/base.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/markdown.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/restructuredtext.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/textile.py 100.00% <100.00%> (ø)
commitizen/commands/bump.py 97.72% <100.00%> (+0.08%) ⬆️
commitizen/commands/changelog.py 99.07% <100.00%> (+0.13%) ⬆️
... and 25 more

📢 Thoughts on this report? Let us know!.

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Hi @noirbizarre for all your amazing contribution! I start to review this one but i've not yet be able to finish it. I'll do so when I have time. Thanks!

commitizen/cli.py Show resolved Hide resolved
commitizen/commands/changelog.py Outdated Show resolved Hide resolved
@noirbizarre noirbizarre force-pushed the feature/template-override branch 3 times, most recently from c2ea42a to 83918e5 Compare January 5, 2023 13:03
docs/customization.md Outdated Show resolved Hide resolved
docs/customization.md Outdated Show resolved Hide resolved
commitizen/changelog.py Outdated Show resolved Hide resolved
@woile
Copy link
Member

woile commented Jan 7, 2023

I like the PR 👍🏻 , though I see an issue, commitizen is using markdown to detect which was the last commit made, and from there create an incremental changelog, without touching the rest of the changelog. How would we guarantee this with the templates feature?

def parse_version_from_markdown(value: str) -> Optional[str]:
if not value.startswith("#"):
return None
m = re.search(defaults.version_parser, value)
if not m:
return None
return m.groupdict().get("version")

One solution would be to add parameter to the config:

[tool.commitizen]
version = "0.3.0"
latest_changelog_version = "0.1.0"

Now, it still worries me, how would commitizen know where to inject the incremental changelog?
At the moment we use an approximation system tied to markdown, if it is a markdown title, then we try to match to a version format.

If we allow for example templates in sphinx, or any other template system, how would commitizen know where to inject the template?

Even with my provided solution using latest_changelog_version a user could potentially add a version as part of a commit (which we have no control). For example, some adds:

fix: update dep X to 0.1.0

without a way to know if it's a title, commitizen would match that, and it would parse that version, potentially injecting the changelog there.

Does it make sense what I'm saying haha?

Could we have some tests with different templates? For example, using AsciiDoc

= Changelog

== 0.1.0

=== Bug Fixes

-  update dep X to 0.1.0

Textile

h1. Changelog

h2. 0.1.0

h3. Bug Fixes

*  update dep X to 0.1.0

Thanks!

@woile
Copy link
Member

woile commented Jan 7, 2023

We could potentially add a command to cz changelog --export-template that prints the template to stdout 🤔

@noirbizarre
Copy link
Member Author

Oh, I didn't notice the template parsing. That is definitively something I need to handle in this PR.
I'll fix all comments and try adding something to handle changelog reverse parsing.

The problem I see with the latest_changelog_version approach is that it is not backward compatible, and we lose some automation.

What about making parse_version_from_markdown a parse_version_from_changelog method on BaseCommitizen ?

This should be backward compatible, Plugin providing their template could override to provide the parsing. For the template from CLI or conf, I can allow providing a string pointing to a custom method, something like:

[tool.commitizen]
template = "my_template.whatever"
changelog_version_parser = "path.to:my_parser"

I like the cz changelog --export-template idea ! Anyone can bootstrap its customized template from the original one. I'll this too 👍🏼

@Lee-W
Copy link
Member

Lee-W commented Jan 23, 2023

Hi @noirbizarre I notice you have some discussion with @woile . Wondering whether this pr is ready for review now?

@noirbizarre
Copy link
Member Author

I rebased it on latest v3, added the changelog --export-template but it's lacking the incremental part (from the discussion 👆🏼).
I hope to be able to deliver it by the end of the week.
However, it is already possible to review what's in it. I'll add the incremental part as an extra commit to help review it later.

@Lee-W
Copy link
Member

Lee-W commented Jan 26, 2023

Got it. Let me focus on #646 first and will get back to this one afterward 💪

@noirbizarre
Copy link
Member Author

noirbizarre commented Oct 16, 2023

I updated the PR including:

  • rebased on style(ruff): enable pyupgrade (UP) rules and fix them all #889 to benefit from Python 3.8+ syntax update (include latest master + ruff pyupgrade enabled)
  • applied all suggestion except the -> None because other methods don't have this syntax (I think this should go in another PR where linters are configured accordingly)
  • all format/Format have been replaced by changelog_format/ChangelogFormat everywhere
  • history squashed to have consistent commits (and so consistent changelog) if merged using Rebase and merge
  • defaults.version_parser has been removed to use the new version_scheme.parser

I'll close all Python 3.8+ syntax update suggestion for easier review (most of them being in #889 or automatically fixed with ruff).
#889 can/should be merged before.

@Lee-W Lee-W added pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check and removed pr-status: wait-for-review pr-status: wait-for-modification labels Oct 17, 2023
@Lee-W
Copy link
Member

Lee-W commented Oct 17, 2023

@noirbizarre I think we're good to merge this one 🎉 but after merging #889, it seems to have some conflicts 😱 Please merge it once you resolve the conflict. Thanks so much for the hard work! this is a really exciting feature!

@noirbizarre
Copy link
Member Author

@Lee-W I think this was a GitHub-only weirdness because I wasn't able to display conflict in the Web UI, and locally I haven't got any (I just did a rebase on the current master without any conflict).

So there it is, PR up to date, waiting for CI to pass, and then it should ready 🚀 🎉

@Lee-W
Copy link
Member

Lee-W commented Oct 17, 2023

hmmm.... not sure what's happening. @noirbizarre do you want to force push to retrigger the github actions again?

…`commitizen.changelog_format` endpoint (Textile, AsciiDoc and RestructuredText)
@noirbizarre
Copy link
Member Author

Done 👍🏼
🤞🏼

@noirbizarre
Copy link
Member Author

noirbizarre commented Oct 17, 2023

This is driving me crazy !!! 😱
Windows builds are failing on pre-commit (not touched in the PR and not related to the PR).
I think the last pre-commit update is broken on Windows runners (for node-based checks). Excluding this version seems to work.
I think I'll do another separate PR to fix the pre-commit test case by using a check which is not relying on NodeJS (will be faster to execute and safer). Most probably a check based on ruff to have both speed (install and exec) and no dependency management to handle in pre-commit.

EDIT: build succeeded 🎉. The pre-commit fix worked.

EDIT2: I tried a lighter change: just updated prettier to the latest version in the test, and it seems to be enough. I don't know whether Windows worker was broken or if there is really an issue with pre-commit, prettier or the Windows runner. Anyway, it worked

@Lee-W Lee-W merged commit 34a7bd5 into commitizen-tools:master Oct 18, 2023
18 checks passed
@noirbizarre noirbizarre deleted the feature/template-override branch October 18, 2023 00:58
@woile
Copy link
Member

woile commented Oct 18, 2023

Awesome news 🎉 🎉 🚀
Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check pr-status: wait-for-review
Projects
None yet
5 participants