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

ci(version): fix poetry project version and version_files config #370

Closed
wants to merge 1 commit into from

Conversation

Lee-W
Copy link
Member

@Lee-W Lee-W commented Apr 9, 2021

Description

We didn't clearly specify which version should be updated in pyproject.toml. After the fix of #361, we need to clearly specify it.

Checklist

  • Add test cases to all the changes you introduce
  • Run ./script/format and ./script/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

Run cz bump and bump the version correctly

Steps to Test This Pull Request

  1. Run cz bump
  2. Check the version in pyproject.toml in poetry and commitizen section

Additional context

@Lee-W Lee-W requested a review from woile April 9, 2021 11:02
@codecov
Copy link

codecov bot commented Apr 9, 2021

Codecov Report

Merging #370 (4e15834) into master (b8bc952) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #370   +/-   ##
=======================================
  Coverage   97.59%   97.59%           
=======================================
  Files          39       39           
  Lines        1373     1373           
=======================================
  Hits         1340     1340           
  Misses         33       33           
Flag Coverage Δ
unittests 97.59% <100.00%> (ø)

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

Impacted Files Coverage Δ
commitizen/__version__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b541add...4e15834. Read the comment docs.

@woile
Copy link
Member

woile commented Apr 9, 2021

Because we are using ci it won't trigger a new pypi deployment. Should we deploy manually or use fix instead?

@woile
Copy link
Member

woile commented Apr 9, 2021

We should probably update the https://github.com/commitizen-tools/commitizen/blob/master/docs/config.md
Unfortunately I think this is a breaking change, we could fix it if we update the config before updating the version numbers I think.

https://github.com/commitizen-tools/commitizen/blob/master/commitizen/commands/bump.py#L179
https://github.com/commitizen-tools/commitizen/blob/master/commitizen/commands/bump.py#L211

What do you think?

@Lee-W
Copy link
Member Author

Lee-W commented Apr 9, 2021

Because we are using ci it won't trigger a new pypi deployment. Should we deploy manually or use fix instead?

Yes, I think it would be better that we manually push this one. Forget to mention it 🤦🏻

@Lee-W
Copy link
Member Author

Lee-W commented Apr 9, 2021

We should probably update the https://github.com/commitizen-tools/commitizen/blob/master/docs/config.md

I agree we should update the documentation as well.

Unfortunately I think this is a breaking change, we could fix it if we update the config before updating the version numbers I think.

https://github.com/commitizen-tools/commitizen/blob/master/commitizen/commands/bump.py#L179
https://github.com/commitizen-tools/commitizen/blob/master/commitizen/commands/bump.py#L211

I'm afraid I did not understand what you mean. Could you describe it a bit more? 🙂

@woile
Copy link
Member

woile commented Apr 9, 2021

I think a solution would be to check for all occurrences when running the regex, that would solve the reported issue and the issue with not modifying the poetry version.

Before we were updating all changes in a file, right? now it seems like we update only one

Edit: Sorry, I didn't mean this PR, I mean the change that forces us to have 2 items in the version_files

@Lee-W
Copy link
Member Author

Lee-W commented Apr 9, 2021

I think what you propose makes sense 👍 I'm thinking of first manually push 2.7.1, then merge this one. After all these step, we could create an issue to fix it. What do you think of the steps?

@woile
Copy link
Member

woile commented Apr 9, 2021

I would only update the version in the tool.poetry. If we fix the version_files, the one we currently have configured should work, right?
I will publish manually to pypi

@Lee-W
Copy link
Member Author

Lee-W commented Apr 9, 2021

Sounds good 👍 Then, I'll close this PR

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.

None yet

2 participants