search

c4urself / bump2version

Version-bump your software with a single command
https://pypi.python.org/pypi/bump2version
MIT License
1.06k stars 134 forks source link

Add parameter to search & replace strictly #132

Closed delbertooo closed 4 years ago

delbertooo commented 4 years ago

The current behavior of search and replace is:

If the pattern of search isn't found, try to search for the default version representation (not the pattern).

This may lead to unwanted or strange replacements. As I debugged such unwanted replacements I found a TODO in your code and decided to implement it. The new behavior (with the option enabled) will be:

If the pattern of search isn't found, there will be an error. The error will be the same error as usual if the version isn't found at all.

The implementation is opt-in and should not alter the current behavior of the program at all. If you don't specify the new option, everything will work as it did before.

I suggest bringing this into Version 1.1.0 if you follow semver.

delbertooo commented 4 years ago

You might want to mention issues, if there are issues affected by this.

florisla commented 4 years ago

Hi delbertooo, did you see #128 ?

Do you think #128 covers your desired behavior, or is your implementation even more strict?

delbertooo commented 4 years ago

Hi, I - in fact - didn't see #128 and #127.

I strongly agree with #127 and going with this would make my feature obsolete. I think I got misled by the fact, that the code of this "fallback feature" is still there but dead now. The side effect, that the existence of the version is checked before replacing makes this code dead (when called from CLI).

TL;DR: I think my code isn't needed if everyone agrees with #127.

Besides that: as you reached the first major version, I would strongly suggest to follow semantic versioning. Introducing #127 and #128 is a breaking change to existing bumpversion derivates and to bump2version itself. I don't think we can declare #127 as a bug, even though it's a pretty useless feature. So IMO this should be part of a new major version with migration notes in a corresponding UPGRADING.md file. But this topic isn't related to the original issue anymore.