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

Switch to versioningit (non-src-layout) #56

Merged
merged 2 commits into from
Sep 8, 2021
Merged

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Jul 9, 2021

This is an alternative to #55 if you don't want to switch to a src/ layout.

@jwodder jwodder added the internal Changes only affect the internal API label Jul 9, 2021
@codecov
Copy link

codecov bot commented Jul 9, 2021

Codecov Report

Merging #56 (f4cd1b3) into master (92f303e) will decrease coverage by 9.33%.
The diff coverage is 100.00%.

❗ Current head f4cd1b3 differs from pull request most recent head b1822bd. Consider uploading reports for the commit b1822bd to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
- Coverage   96.05%   86.72%   -9.34%     
==========================================
  Files          13       13              
  Lines        1319     1273      -46     
==========================================
- Hits         1267     1104     -163     
- Misses         52      169     +117     
Flag Coverage Δ
unittests 86.72% <100.00%> (-9.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dandischema/__init__.py 100.00% <100.00%> (ø)
dandischema/datacite.py 12.50% <0.00%> (-82.40%) ⬇️
dandischema/tests/test_datacite.py 36.50% <0.00%> (-63.50%) ⬇️
dandischema/tests/test_models.py 95.41% <0.00%> (-0.39%) ⬇️
dandischema/consts.py 100.00% <0.00%> (ø)
dandischema/metadata.py 96.79% <0.00%> (ø)
dandischema/models.py 93.92% <0.00%> (+0.14%) ⬆️

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 92f303e...b1822bd. Read the comment docs.

@jwodder jwodder marked this pull request as ready for review July 9, 2021 03:55
# Same format as versioneer
distance = "{version}+{distance}.{vcs}{rev}"
dirty = "{version}+{distance}.{vcs}{rev}.dirty"
distance-dirty = "{version}+{distance}.{vcs}{rev}.dirty"
Copy link
Member

Choose a reason for hiding this comment

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

since it would be a common usecase AFAIK -- why versiongit doesn't just default to it (and also for above default-version which sounds like something to be within the same config section IMHO) and thus not require all this boilerplate in every project?

Copy link
Member Author

@jwodder jwodder Jul 9, 2021

Choose a reason for hiding this comment

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

versioningit already has a default set of formats (listed at the end of this section). I just gave different ones here so that the versioning would continue to be consistent with versioneer. As for default-version, that's for recovering from errors, which I believe the default response to should be the installer failing.

@@ -1,10 +1,25 @@
[build-system]
# Setuptools version should match setup.py; wheel because pip will insert it noisily
requires = ["setuptools >= 38.3.0", "wheel"]
requires = ["setuptools >= 42.0.0", "versioningit ~= 0.1.0", "wheel"]
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, didn't try yet, but looks really neat in that it indeed seems to avoid needing all the copied code etc as versioneer does. That though then makes projects rely on the 'stability' of versiongit but indeed ~= (or even == for paranoids) pinning should provide remedy

@yarikoptic
Copy link
Member

@satra WDYT -- should we give it a try?

@satra
Copy link
Member

satra commented Jul 20, 2021

sure - let's try it out.

@yarikoptic
Copy link
Member

sorry , this one fell off the radar. @jwodder - could you please rebase so we get a fresh sweep of testing, and then lets merge?

@yarikoptic
Copy link
Member

I am sorry -- radar was still "off". Once more @jwodder and feel free to merge it if no CI errors can be coming from changes in the PR

@yarikoptic
Copy link
Member

thank you @jwodder , all green, let's give it a shot!

@yarikoptic yarikoptic merged commit 1054377 into master Sep 8, 2021
@yarikoptic yarikoptic deleted the versioningit2 branch September 8, 2021 14:46
@yarikoptic yarikoptic mentioned this pull request Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes only affect the internal API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants