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

Force constant version date length #1995

Merged
merged 3 commits into from
May 3, 2022

Conversation

epassaro
Copy link
Member

@epassaro epassaro commented Apr 30, 2022

Description

Currently, version names have the form tardis-YYYY.MM.DD.B with:

Y: year
M: month
D: day
B: build number (if build number is >0, otherwise version is just YYYY.MM.DD)

But for months from 1-9 and days from 1-9 the version is resolved as YYYY.M.D.

With this patch the number of digits is always 8.

Motivation and context

How has this been tested?

  • Testing pipeline.
  • Other.

Locally run with the tardis env activated: python .ci-helpers/get_next_version.py

Examples

Type of change

  • Bug fix.
  • New feature.
  • Breaking change.
  • None of the above.

Checklist

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
    • (optional) I have built the documentation on my fork following the instructions.
  • I have assigned and requested two reviewers for this pull request.

@epassaro epassaro marked this pull request as draft April 30, 2022 14:09
@epassaro epassaro force-pushed the ci/fixed-ver-length branch from d775e31 to 6c2f344 Compare April 30, 2022 14:10
@epassaro epassaro marked this pull request as ready for review April 30, 2022 14:11
@codecov
Copy link

codecov bot commented Apr 30, 2022

Codecov Report

Merging #1995 (814aec1) into master (ae19f26) will not change coverage.
The diff coverage is n/a.

❗ Current head 814aec1 differs from pull request most recent head 748bf59. Consider uploading reports for the commit 748bf59 to get more accurate results

@@           Coverage Diff           @@
##           master    #1995   +/-   ##
=======================================
  Coverage   59.99%   59.99%           
=======================================
  Files          70       70           
  Lines        8111     8111           
=======================================
  Hits         4866     4866           
  Misses       3245     3245           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@andrewfullard
Copy link
Contributor

I feel like there's a better way to do this with a datetime transformation to ISO format? https://docs.python.org/3/library/datetime.html#datetime.date.isoformat

@epassaro epassaro changed the title Force fixed digit number of digits in version number Force constant version date length May 3, 2022
@epassaro
Copy link
Member Author

epassaro commented May 3, 2022

I feel like there's a better way to do this with a datetime transformation to ISO format? https://docs.python.org/3/library/datetime.html#datetime.date.isoformat

Ok, then I should use replace method to switch dashes for dots?

@epassaro
Copy link
Member Author

epassaro commented May 3, 2022

@andrewfullard done

@@ -2,7 +2,6 @@

import sys
from setuptools_scm import version_from_scm
from setuptools_scm.version import guess_next_date_ver
Copy link
Member Author

Choose a reason for hiding this comment

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

Unused import

@@ -2,7 +2,6 @@

import sys
from setuptools_scm import version_from_scm
from setuptools_scm.version import guess_next_date_ver
Copy link
Member Author

Choose a reason for hiding this comment

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

Unused import

iso_date = date(*release).isoformat()
iso_date = iso_date.replace("-",".")

version = f"{iso_date}.{str(build)}"
version = version.rstrip(".0") if version.endswith(".0") else version
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

@epassaro epassaro merged commit fb853fa into tardis-sn:master May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants