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

Fix for reporting dirty when the commit count is zero #44

Merged
merged 5 commits into from
Apr 24, 2018

Conversation

jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Apr 24, 2018

Also updates one outdated ref. Addresses #43

@jlstevens
Copy link
Contributor Author

@ceball Looks like the tests will all go green now.

@ceball
Copy link
Member

ceball commented Apr 24, 2018

I can't review the actual change to autover (I looked but am slightly confused about Version.commit_count, Version. _commit_count, Version.fetch()._commit_count). But the change seems to work so I'd be happy to see it merged.

@ceball ceball merged commit 3635e09 into master Apr 24, 2018
@jlstevens
Copy link
Contributor Author

Version.commit_count is the public property, Version._commit_count is the internal state that caches the value after calling git describe with fetch() and Version.fetch()._commit_count is the internal value after a fetch.

It is just a simple way to avoid calling git describe excessively which is expensive to do. Using the properties is always safe.

ceball added a commit to holoviz/param that referenced this pull request Apr 24, 2018
  * Allow develop_install to work (holoviz-dev/autover#41)

  * Failed to report 'dirty' for commit count of 0 (holoviz-dev/autover#44)
ceball added a commit to holoviz/param that referenced this pull request Apr 24, 2018
* Allow develop_install to work (holoviz-dev/autover#41)

* Failed to report 'dirty' for commit count of 0 (holoviz-dev/autover#44)
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.

None yet

2 participants