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

Make 'part' optional (but not forbidden) when new_version is specified #51

Closed
wants to merge 6 commits into from

Conversation

florisla
Copy link
Collaborator

@florisla florisla commented Dec 27, 2018

This is an alternative implementation to #31 which tries to be backwards compatible to existing uses
which supply a part argument just because it's required by the CLI.

Closes #22

I think this will only break if the part argument is actually present on disk as a file. That should be low probability since most files in the root will start with a dot or have an extension.

It's up to the maintainers to choose if #31 is preferred (it's definitely cleaner and less ambiguous) or if it's worth retaining backwards compatibility.

Future work would be to update the documentation, and to add a deprecation warning when a useless part is supplied. (done now)

Includes a test to verify that the DeprecationWarning actually occurs.

In two other tests, the warnings are suppressed to not litter the output
of pytest.
It's 'file-like' if it includes a dot, slash or backslash.
Unless of course there is matching part configuration (which
includes the dot/slash/backslash).

Without this change, test 'non_existing_file' fails because
bump2version considers the supplied non-existing path to be an
unnecessary [part] argument and only produces a
DeprecationWarning for it.

While of course we want a proper FileNotFoundError (IOError) if we
supply an argument that looks like a file and can not be found on
disk.

Parametrize the 'non existing file' test to check all cases of dot,
slash, backslash.
This avoids DeprecationWarning bubbling up to the pytest output.
@florisla
Copy link
Collaborator Author

florisla commented Jan 10, 2019

I think this will only break if the part argument is actually present on disk as a file. That should be low probability since most files in the root will start with a dot or have an extension.

If you supply a part when it's not required (new_version is specified), this is handled like this

  • it is an actual part: show a DeprecationWarning
  • it's not an actual part, but it's present on disk: it is treated as a file
  • it's not a part, not present on disk, but looks like a path (includes dot, slash or backslash): it's treated as a file path (resulting in IOError)
  • not a part, not on disk, does not look like a file: it is treated as a bogus [part]: show DeprecationWarning

I think this works pretty well now. Tests are in place and I've updated the documentation.

Not sure if we should use PendingDeprecationWarning instead of DeprecationWarning (see docs).

@florisla florisla changed the title WIP: Make 'part' optional (but not forbidden) when --new-version is supplied Make 'part' optional (but not forbidden) when --new-version is supplied Jan 10, 2019
@florisla florisla changed the title Make 'part' optional (but not forbidden) when --new-version is supplied Make 'part' optional (but not forbidden) when new_version is supplied Jan 10, 2019
@florisla florisla changed the title Make 'part' optional (but not forbidden) when new_version is supplied Make 'part' optional (but not forbidden) when new_version is specified Jan 10, 2019
@florisla
Copy link
Collaborator Author

Going to retry this in a new PR.

@florisla florisla closed this Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

part should not be required if supplying --new-version
1 participant