search

labscript-suite / labscript-utils

Shared modules used by the 𝘭𝘒𝘣𝘴𝘀𝘳π˜ͺ𝘱𝘡 𝘴𝘢π˜ͺ𝘡𝘦. Includes a graphical exception handler, debug tools, configuration management, cross platform filepath conversions, unit conversions and custom GUI widgets.
http://labscriptsuite.org
Other
2 stars 45 forks source link

Restore check_version() #65

Closed zakv closed 3 years ago

zakv commented 4 years ago

Resolves #62

This PR reverts d7230c59cdfee9ca5f87feef64fa9fe00e793ff0 and replaces distutils.version.LooseVersion with packaging.version as suggested in #62.

zakv commented 4 years ago

Just to double check, LooseVersion('3.0.0rc2.dev11+g8803447') < LooseVersion('3.0.0') gave False, but packaging.version.parse('3.0.0rc2.dev11+g8803447') < packaging.version.parse('3.0.0') gives True. Is that new behavior desired?

philipstarkey commented 4 years ago

I think the new behaviour is what we want. Release candidates/development versions come before actual releases. The only thing I'm uncertain of is whether our setuptools_scm configuration actually roles over to a new minor version once we create a release branch. If it doesn't then new development would still be on the old version and appear older in check_version comparisons even though it's actually newer.

zakv commented 4 years ago

Yep I think that is an issue. That '3.0.0rc2.dev11+g8803447' version string above is what I have after branching off of the most recent commit from lyse's master branch and then committing some changes. I would have expected that version to count as greater than 3.0.0 and 3.0.0rc2, but the opposite is true.

Maybe this means that the setuptools_scm configuration needs to be changed? Alternatively I could revert the code back to using LooseVersion.

chrisjbillington commented 4 years ago

Branching off release branches should never be done without a tag at the branch point incrementing the minor number. That's our plan, anyway.

Doesn't that mean this distinction is moot?

zakv commented 4 years ago

Are you saying that there should be a '3.1.0' tag in the lyse repo right now which would make feature branches be versioned '3.1.0...' instead of '3.0.0...?

philipstarkey commented 3 years ago

@zakv I've made a change to how setuptools_scm does version numbers in #70. Does this change the observed behaviour for you in this PR? (you may need to rerun the editable pip install command with --no-deps to get a new version...can't remember if we fixed the need for rerunning that after update or not now)

EDIT: You don't need to rerun the pip install. __version__.py detects the .git folder and uses setuptools_scm instead of package metadata (which is the point of fixing setuptool_scm config)

zakv commented 3 years ago

Yep! Without #70 labscript_utils.__version__ gives '3.0.0rc2.dev39+g91b7562', but with #70 that gives '3.1.0.dev44+g570f587'. I believe that makes the ordering between release versions and commits on master make sense; commits since '3.0.0' are '3.1.0.dev...' so they are ordered after '3.0.0' but before '3.1.0'.

Looks like the 'rc' part isn't included in the version string prodcued with #70. Does that matter?

philipstarkey commented 3 years ago

Yep, the missing RC is expected because we have not tagged any releases on master after the last release branch. If you wanted to test that locally, you could add a tag like v3.1.0rc1 and see what happens. Don't forget to delete the tag before your next push though if you do that!