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

fix: fix multiple versions bumps when version changes the string size #374

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

Conversation

Copy link
Contributor

@jaysonsantos jaysonsantos commented Apr 17, 2021 •
edited
Loading

Description

As I've commented here the bug fix that fixes my bug also introduces a new one.
If you have a file that has many version = 1.0.9 and you bump to 1.1.10 every time you bump the version, the file increases 1 byte, and re.findall keeps a reference to the old string making every iteration point to a character back like (version, versio, versi, vers, and etc) to the point where it would move to previous likes and the check that makes sure that the current version is found on the current line fails.

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

All versions should be bumped no matter the size of the file and occurrences of a regex

Steps to Test This Pull Request

1 - Have a file with at least 8 version = 1.0.9 lines
2 - Bump version to 1.1.10 using file:version as the regex

Additional context

#372
#361

woile and Lee-W reacted with heart emoji
Copy link

codecov bot commented Apr 17, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@b94418b). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph 

@@ Coverage Diff @@
## master #374 +/- ##
=========================================
 Coverage ? 97.60% 
=========================================
 Files ? 39 
 Lines ? 1378 
 Branches ? 0 
=========================================
 Hits ? 1345 
 Misses ? 33 
 Partials ? 0
Flag Coverage Δ
unittests 97.60% <100.00%> (?)

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

Impacted Files Coverage Δ
commitizen/bump.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 b94418b...f2cbf34. Read the comment docs.

Copy link
Member

woile commented Apr 18, 2021

Looks good to me, feel free to merge @Lee-W

@Lee-W Lee-W merged commit 127d1b0 into commitizen-tools:master Apr 19, 2021
Copy link
Member

Lee-W commented Apr 19, 2021

This one is interesting. 🤔 Didn't notice this might happen. Thanks, @jaysonsantos 👍👍👍

@jaysonsantos jaysonsantos deleted the fix-change-versions-with-different-string-lengths branch April 19, 2021 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lee-W Lee-W Lee-W approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /