Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

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

jaysonsantos
Copy link
Contributor

@jaysonsantos jaysonsantos commented Apr 17, 2021

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

@codecov
Copy link

codecov bot commented Apr 17, 2021

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.

@woile
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
@Lee-W
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants