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

test_result_logger: accept release version numbers #421

Closed
wants to merge 2 commits into from
Closed

test_result_logger: accept release version numbers #421

wants to merge 2 commits into from

Conversation

ArchangeGabriel
Copy link

When building on release, the output version is e.g. 0.8.0, without dev part.
Thus, this commit makes the dev part optional, allowing it to match both cases.
Fixes GH-420.

@welcome
Copy link

welcome bot commented Oct 31, 2019

💖 Thanks for opening your first pull request here! 💖

When building on release, the output version is e.g. 0.8.0, without dev part.
Thus, this commit makes the dev part optional, allowing it to match both cases.
Fixes GH-420.

Signed-off-by: Bruno Pagani <bruno.n.pagani@gmail.com>
Copy link
Member

@Freso Freso left a comment

Choose a reason for hiding this comment

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

Seems good. The only thing I would request is a test case of two where the current test would fail but the change in the test code makes the cases now pass.

@Freso
Copy link
Member

Freso commented Oct 31, 2019

Ideally there should be at least four test cases, one for each of the situations described at https://github.com/pypa/setuptools_scm/#default-versioning-scheme

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Copy link
Collaborator

@Freso @ArchangeGabriel I think the following RegEX matches all the (4) cases:

[\d]+\.[\d]+\.[\d]+(\+d\d{8}|\.dev[\w\.\+]+)? \(internal logger\)

JoeLametta added a commit that referenced this pull request Dec 3, 2019
Initial work by ArchangeGabriel: #421

Fixes #420.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta JoeLametta closed this Dec 3, 2019
hydrian pushed a commit to hydrian/whipper that referenced this pull request Dec 18, 2019
Initial work by ArchangeGabriel: whipper-team#421

Fixes whipper-team#420.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: hydrian <ben.tyger@tygerclan.net>
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.

Test failure when building a release
3 participants