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

get_version_from_string missing doc string/surprising regex #3

Open
tomato42 opened this issue Mar 27, 2018 · 1 comment
Open

get_version_from_string missing doc string/surprising regex #3

tomato42 opened this issue Mar 27, 2018 · 1 comment
Labels
enhancement New feature or request

Comments

@tomato42
Copy link

https://github.com/ansasaki/symbol_version/blob/f441d3446bcb325df9a3ae440313d30da2646952/scripts/symbol_version.py#L55-L73

  1. the method is missing a doc string
  2. is it expected that the error does not halt execution?
  3. the warnings talk about numeric versions but the regex will also match alphanumeric versions, like ab.dd.pre1
  4. I don't know how strictly those should be semantic versions, but for those, there's a setuptools parse_version() function (not saying it should be used, that depends on 1. 😃 )
  5. that loop can be replaced by list comprehension:
return [int(i) for i in m]
@ansasaki
Copy link
Owner

  1. A docstring was added
  2. In case of error an exception is raised
  3. In the beginning it was supposed to support different version styles (using letters , like 1.0.a). But this was changed because it makes difficult to make the version bump (that is the reason for issue Allow user to configure version bump strategy #20)
  4. Looks good, maybe I'll switch to this implementation
  5. Now I know how to use list comprehensions (and sometimes I remember to use generators)

@ansasaki ansasaki added the enhancement New feature or request label Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants