Skip to content

repo.tag() only accepts 'ref/tags/tagname' instead of just 'tagname' #1261

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

Closed
bytefluxio opened this issue Jun 2, 2021 · 2 comments · Fixed by #1264
Closed

repo.tag() only accepts 'ref/tags/tagname' instead of just 'tagname' #1261

bytefluxio opened this issue Jun 2, 2021 · 2 comments · Fixed by #1264

Comments

@bytefluxio
Copy link
Contributor

bytefluxio commented Jun 2, 2021

The param comment for tag() in base.py mentions 0.1.5 and tags/0.1.5 as valid inputs, however the call to TagReference calls the inherited constructor (Reference in reference.py) with the default check_path=True, which returns a ValueError, if the tag/path given doesnt start with self._common_path_default + '/'.

Either the param comment needs to be adjusted or the tag parameter cleaned up so that it only returns the tag value to Reference.__init__.

pycharm64_UlSdzGazov
pycharm64_nnw8DMULi2

@Byron
Copy link
Member

Byron commented Jun 3, 2021

Thanks for letting me know - I see that the current state is not correct.

Since tag() sits on the Repo type I would love to actually behave as advertised as it reflects typical usage. Unfortunately the TagReference type might not work correctly if its name is not the complete path to the reference, so remving this check probably won't do.

Instead the name should be resolved to a path which might at least need a call to the git program. Alternatively one could probably naively fix it by traversing all tags and finding a matching tag name. All this though seems quite time consuming and I wonder if there is a more elegant way.

If you consider a PR, for now fixing the docs to reflect the actual behaviour would already help to reduce surprises, and we can keep an issue here to propose enhancing this to support tag names as well.

What do you think?

@bytefluxio
Copy link
Contributor Author

I'll get right on it. :)

I'm quite new to python though, so be harsh and truthful with your remarks.
However I'm already stumped regarding the positioning of private methods. They seem to be all over the place. I can't make out an order. I'll just place the one I intend to make right below where I'm using it and you can then point me to the right place :P

@bytefluxio bytefluxio mentioned this issue Jun 3, 2021
@Byron Byron added this to the v3.1.18 - Bugfixes milestone Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants