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.

Sign up for GitHub

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

Jump to bottom

When search is not found, only fall back to original_version if search is not customized #128

Merged
merged 5 commits into from
Feb 20, 2020
Merged

When search is not found, only fall back to original_version if search is not customized #128

merged 5 commits into from
Feb 20, 2020

Conversation

florisla
Copy link
Collaborator

Closes #127

@florisla
Cleanup: remove capsys fixture where it's not used
323decd
@florisla
Cleanup: derive all custom exceptions from one base class
b43068f 
This allows downstream users (calling main() as a library) to catch
a single exception type.
@florisla
Add unit test: don't touch source files if search is not found
d71a906 
This currently fails.
@florisla
Fix: only match on original_version if search isn't customized
9f82bbd
@florisla
Refactor: Use custom VersionNotFoundException instead of ValueError
eab75c5 
Before, this was an AssertionError but that's no good either -- assert() can be
disabled in Python (-O flag) and that would kill this functionality altogether.
@florisla
Copy link
Collaborator Author

florisla commented Jan 27, 2020 •
edited

I've tried removing the fallback to original_version altogether, but that fails some cases.

The scenario is:

  • a custom serialize pattern introduces extra (new) data into the serialized version string, e.g. from an $ENV_VARIABLE.
  • as a result, the serialized version string is not currently present in the source file, but the original current_version string is.
  • without falling back to original_version, the replacement can not occur.

Current tests which exposed this are test_bump_version_environment, test_distance_to_latest_tag_as_part_of_a_new_version, test_multi_file_configuration2.

I think this current approach is an improvement nonetheless. It will prevent the incorrect operation shown in #127 and still supports current usage.

@ekohl ekohl merged commit 648036b into c4urself:master Feb 20, 2020
@ekohl
Copy link
Collaborator

ekohl commented Feb 20, 2020

Looks like a good improvement, thanks!

@florisla florisla mentioned this pull request Feb 27, 2020
Closed
@florisla florisla deleted the no-search-fall-back branch February 28, 2020 07:35
@kyluca kyluca mentioned this pull request Oct 27, 2020
Closed
@florisla florisla added this to the v1.0.1 milestone Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

@c4urself c4urself Awaiting requested review from c4urself

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
v1.0.1
Development

Successfully merging this pull request may close these issues.

Don't fall back to original_version if search pattern is not found
2 participants
@florisla @ekohl