ansasaki / abimap

A helper for library maintainers to use symbol versioning
MIT License
7 stars 1 forks source link

get_version_from_string missing doc string/surprising regex #3

Open tomato42 opened 6 years ago

tomato42 commented 6 years ago

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. :smiley: )
  5. that loop can be replaced by list comprehension:
    return [int(i) for i in m]
ansasaki commented 6 years ago
  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 #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)