Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

DRAFT: feat(provider): add new maven provider for handling versions from pom.xml#963

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
rshmhrj wants to merge1 commit intocommitizen-tools:master
base:master
Choose a base branch
Loading
fromrshmhrj:feat/add-maven-provider

Conversation

rshmhrj
Copy link

@rshmhrjrshmhrj commentedJan 17, 2024
edited by Lee-W
Loading

Description

Adding a new maven provider
Will also be looking into adding a maven version scheme (SemVer + Qualifier) for use in java projects

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

Expected behavior

Oncz init have the option to choosemvn to read and write versions to thepom.xml file

Versioning ref:https://octopus.com/blog/maven-versioning-explained

Steps to Test This Pull Request

  1. Open a java project with apom.xml in the root dir
  2. Make tag if not already present (git tag v0.0.1-SNAPSHOT)
  3. Activate poetry venvsource /path_to_poetry_virtualenvs/commitizen-4BNoRlBJ-py3.12/bin/activate
  4. Run cz init directlypython3 /path_to_commitizen_repo/commitizen/cli.py init

mvn is now an option to choose from

image

Can currently successfully add with normal SemVer (minus qualifiers)
image

Additional context

marcosschroh reacted with thumbs up emoji
@rshmhrjrshmhrj changed the titlefeat(provider): add new maven provider for handling versions from pom.xmlDRAFT: feat(provider): add new maven provider for handling versions from pom.xmlJan 17, 2024
@codecovCodecov
Copy link

codecovbot commentedJan 17, 2024
edited
Loading

Codecov Report

Attention: Patch coverage is94.73684% with1 line in your changes missing coverage. Please review.

Project coverage is 97.41%. Comparing base(120d514) to head(754d498).
Report is 538 commits behind head on master.

Files with missing linesPatch %Lines
commitizen/commands/init.py80.00%1 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##           master     #963      +/-   ##==========================================+ Coverage   97.33%   97.41%   +0.07%==========================================  Files          42       56      +14       Lines        2104     2360     +256     ==========================================+ Hits         2048     2299     +251- Misses         56       61       +5
FlagCoverage Δ
unittests97.41% <94.73%> (+0.07%)⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

@noirbizarre
Copy link
Member

noirbizarre commentedMar 5, 2024
edited
Loading

Glad to see people are finding the version provider useful.

BUT, it has been design so extra provider can be created as separate plugins to avoid putting all the maintenance charge to the team. I don't know if the team is willing to support lots of providers for stacks they might not use and won't be able to maintain.

My advice would be the following: publish this provider as a standalonecommitizen-maven package and register it onthis documentation page so people can easily discover it. This way it will be easier to maintain and update independently. From your point of view, it won't change a thing: same code, just published as a separate package, you just need to install it withcommitizen and it works with the python package, the pre-commit hook withadditional_dependencies or the action withextra_requirements.

@Lee-W@woile What's your opinion on this ?

Lee-W and marcosschroh reacted with thumbs up emoji

@woile
Copy link
Member

I agree, extra providers should be provided on a different repo 🙏🏻

Lee-W reacted with thumbs up emoji

@noirbizarre
Copy link
Member

noirbizarre commentedMar 10, 2024
edited
Loading

I have been thinking a lot about this one.

I think we need to apply our own recommendations, given we already supportnpm,composer andcargo (I don't include Python providers in this list because commitizen is a Python tool, installable with Python tooling and natively able to read its configuration from officialpyproject.toml), to be consistent we should either apply the same decision to those 3 providers and publish them as external packages, either accept this PR.

Furthermore, I see benefit in externalizing those 3 providers:

  • dogfooding: we apply to ourselves what we recommend to other, meaning that most of the time, issues on the pattern would be already fixed
  • samples: anyone willing to craft a provider would have 3 examples of existing external providers
  • clarity: this thread shows that it is hard to draw the line between what should be accepted and what should be external. This would give a single rule to apply
  • fairness: hard to tell, no we won't accept this provider while we already have 3 non-python providers

WDYT ?

@rshmhrj
Copy link
Author

@noirbizarre Thanks for going through the thought process behind externalizing a provider -- I understand the increased burden and stress of having to make updates to a provider you don't have any experience or proficiency in.

I have a few semi-related questions:

  • what is your recommendation for new versions (e.g. SemVer + qualifier (SV+Q) like3.2.1-SNAPSHOT) and where they should be developed and maintained?
  • ideally the maven provider would be able to work hand-in-hand with that SV+Q version -- should they both be in the same project?
  • if mvn and SV+Q are bundled, would it be possible to simultaneously use a different provider with SV+Q?

@Lee-W
Copy link
Member

I have been thinking a lot about this one.

I think we need to apply our own recommendations, given we already supportnpm,composer andcargo (I don't include Python providers in this list because commitizen is a Python tool, installable with Python tooling and natively able to read its configuration from officialpyproject.toml), to be consistent we should either apply the same decision to those 3 providers and publish them as external packages, either accept this PR.

Furthermore, I see benefit in externalizing those 3 providers:

* dogfooding: we apply to ourselves what we recommend to other, meaning that most of the time, issues on the pattern would be already fixed* samples: anyone willing to craft a provider would have 3 examples of existing external providers* clarity: this thread shows that it is hard to draw the line between what should be accepted and what should be external. This would give a single rule to apply* fairness: hard to tell, no we won't accept this provider while we already have 3 non-python providers

WDYT ?

I think this is definitely something we want. We probably would want to remove them after 4.0 as this is a breaking change, but we could start making these providers as standalone packages and add deprecation warning in the main repo

@Lee-WLee-W assignedLee-W andnoirbizarre and unassignedLee-WMar 30, 2024
@Lee-W
Copy link
Member

@noirbizarre Thanks for going through the thought process behind externalizing a provider -- I understand the increased burden and stress of having to make updates to a provider you don't have any experience or proficiency in.

I have a few semi-related questions:

* what is your recommendation for new versions (e.g. SemVer + qualifier (SV+Q) like `3.2.1-SNAPSHOT`) and where they should be developed and maintained?

Do you mean the version ofcommitizen-maven? I think so.

* ideally the maven provider would be able to work hand-in-hand with that SV+Q version -- should they both be in the same project?

I though maven provider is only for finding where maven defines the version. Or am I missing anything?

* if mvn and SV+Q are bundled, would it be possible to simultaneously use a different provider with SV+Q?

@noirbizarre Not sure whether we have this support at this moment. 🤔 but may be something we can work on

@marcosschroh
Copy link
Contributor

Thanks for the work@rshmhrj . Are you planning to create a package for this new provider? Without the provider, how are you changing the version in thepom.xml? In my case I am addingmetadata to theversion attribute in thepom.xml and then using it in the commitizen config, for example:

<?xml version="1.0" encoding="UTF-8"?><projectxmlns="http://maven.apache.org/POM/4.0.0"xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">    <modelVersion>4.0.0</modelVersion>    <groupId>org.example</groupId>    <artifactId>my-sdk</artifactId>    <versioncommitizen="true">1.0.0</version>    ...</project>

and then in.cz.toml

version_files = ["pom.xml:commitizen",]

@Lee-W
Copy link
Member

as it is still marked as draft and the CI has not changed. I'll change it to draft

@noirbizarre
Copy link
Member

Given the comments above, here's what I propose:

  • merge this as soon as CI is passing
  • start preparing external providers as standalone libraries for the next major
  • maybe plan a transitional release marking the built-in one meant to be extracted as deprecated

Perhaps we should make an org project to plan this properly and not forget anything

Lee-W reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@woilewoileAwaiting requested review from woilewoile is a code owner

@Lee-WLee-WAwaiting requested review from Lee-WLee-W is a code owner

@noirbizarrenoirbizarreAwaiting requested review from noirbizarrenoirbizarre is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@noirbizarrenoirbizarre

Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@rshmhrj@noirbizarre@woile@Lee-W@marcosschroh

[8]ページ先頭

©2009-2025 Movatter.jp