Skip to content

Fix version numbering#476

Merged
chmreid merged 3 commits intomasterfrom
chmreid-fix-version-numbering
Nov 5, 2019
Merged

Fix version numbering#476
chmreid merged 3 commits intomasterfrom
chmreid-fix-version-numbering

Conversation

@chmreid
Copy link
Copy Markdown
Contributor

@chmreid chmreid commented Oct 29, 2019

This fixes inconsistent version number behavior as mentioned in #475 - also closes #475.

Before merging this pull request, the version number for the master branch should really be changed from "6.5.0" to something like "6.5.0dev" "6.6.0dev" (or similar) that will indicate it is different from the tagged version 6.5.0.

@amarjandu
Copy link
Copy Markdown
Contributor

can we bump up the patch version of it? vs having 6.5.0dev

Copy link
Copy Markdown
Contributor

@amarjandu amarjandu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, this version file always gets messed up >.>

@chmreid
Copy link
Copy Markdown
Contributor Author

chmreid commented Oct 29, 2019

can we bump up the patch version of it?

Do you mean 6.6.0dev or 5.6.1?

Copy link
Copy Markdown
Contributor

@DailyDreaming DailyDreaming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this.

Just for reference, the old way of distinguishing dev was a 0.0.0 version. I think at some point Jesse and Hannes decided to deviate from this (I'm not sure it's documented anywhere or that the old convention was either).

You have to go back a bit: https://github.com/HumanCellAtlas/dcp-cli/blob/fdb716e69491c2b797883966d1541ab338ca82af/hca/version.py

You should modify this line to catch versions ending in dev instead of catching 0.0.0:

if __version__ == '0.0.0':

That said, keeping the version as an rc and specifying it with dev sounds good to me.

@jessebrennan
Copy link
Copy Markdown
Contributor

jessebrennan commented Oct 31, 2019

@DailyDreaming I've made no (*intentional) changes to how the versioning was done... I have done a couple releases, but I just followed the procedure in the DCP Ops Wiki. I find your vague and unwarranted suspicion is off-putting and befuddling.

@amarjandu
Copy link
Copy Markdown
Contributor

@DailyDreaming I've made no changes to how the versioning was done... I have done a couple releases, but I just followed the procedure in the DCP Ops Wiki. I find your vague and unwarranted suspicion is off-putting and befuddling.

Screen Shot 2019-10-31 at 9 52 03 AM

If this fix means that we don't have to keep changing this file back to 0.0.0 that would be perfect. I've committed this file before to a random version number, it always manages to sneak into the commit.

@DailyDreaming
Copy link
Copy Markdown
Contributor

DailyDreaming commented Nov 1, 2019

@jessebrennan I'm sure it was accidental (and something I've done before too). No suspicion or otherwise intended! As Amar mentioned, the version in master was kept at 0.0.0 but it gets rewritten and accidentally included in commit changes, so we used to have to be wary and make certain that it was changed back. I've changed it accidentally too, and because there's no documentation around it, I'm sure you (or someone else) accidentally changed it without realizing it.

I think setting it to the actual version with an rc or dev modifier makes a lot more sense.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 1, 2019

Codecov Report

Merging #476 into master will increase coverage by 0.7%.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #476     +/-   ##
========================================
+ Coverage   85.19%   85.9%   +0.7%     
========================================
  Files          41      41             
  Lines        1844    1986    +142     
========================================
+ Hits         1571    1706    +135     
- Misses        273     280      +7
Impacted Files Coverage Δ
hca/cli.py 63.39% <0%> (ø) ⬆️
hca/version.py 100% <100%> (ø) ⬆️
hca/util/__init__.py 89.23% <0%> (+2.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca60108...811ca93. Read the comment docs.

@chmreid
Copy link
Copy Markdown
Contributor Author

chmreid commented Nov 1, 2019

Set version to 6.6.0dev and updated the version check function to see if version ends with dev

@chmreid chmreid merged commit fd70455 into master Nov 5, 2019
chmreid added a commit that referenced this pull request Nov 5, 2019
chmreid added a commit that referenced this pull request Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect version number behavior

5 participants