Skip to content

Match more than one digit for version numbers#1889

Merged
thomas-zahner merged 1 commit intolycheeverse:masterfrom
eread:eread/test-changes-to-docker-image-tagging
Oct 29, 2025
Merged

Match more than one digit for version numbers#1889
thomas-zahner merged 1 commit intolycheeverse:masterfrom
eread:eread/test-changes-to-docker-image-tagging

Conversation

@eread
Copy link
Contributor

@eread eread commented Oct 27, 2025

As suggested here: #1833 (comment) by @thomas-zahner, let's test the result of #1881.

@eread eread force-pushed the eread/test-changes-to-docker-image-tagging branch 3 times, most recently from 83fe33e to 68cbed0 Compare October 28, 2025 02:25
@eread
Copy link
Contributor Author

eread commented Oct 28, 2025

@thomas-zahner Ok, I think we need one more minor change based on this commit: 68cbed0, and these runs:

Basically, /d will match only a single digit, whereas the minor version number for Lychee right now is more than one digit, so the regular expression as it currently is doesn't quite catch it.

I'll refactor this PR to just make that change, and I think the Docker images will be properly tagged for the next release!

@eread eread force-pushed the eread/test-changes-to-docker-image-tagging branch from 68cbed0 to 5a86bde Compare October 28, 2025 02:37
@eread eread changed the title Test changes to Docker image tagging Match more than one digit for the minor version number Oct 28, 2025
@eread
Copy link
Contributor Author

eread commented Oct 28, 2025

@thomas-zahner Could you review? I believe this change is required (see: #1889 (comment)).

Copy link
Member

@thomas-zahner thomas-zahner left a comment

Choose a reason for hiding this comment

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

Glad to hear that you could test the changes in your local repository and thank you for testing and submitting this PR.

@eread eread force-pushed the eread/test-changes-to-docker-image-tagging branch from 5a86bde to 6c48cfc Compare October 28, 2025 22:59
@eread
Copy link
Contributor Author

eread commented Oct 28, 2025

Glad to hear that you could test the changes in your local repository and thank you for testing and submitting this PR.

Thanks for the review @thomas-zahner 🙇🏻

I thought you might ask for all three parts of the version number to be similarly handled.

Here's a commit for testing: 6c48cfc

Here's the test run with results:

I'll now refactor the PR to just have the required changes.

@eread eread force-pushed the eread/test-changes-to-docker-image-tagging branch from 6c48cfc to d980255 Compare October 28, 2025 23:15
@eread eread changed the title Match more than one digit for the minor version number Match more than one digit for version numbers Oct 28, 2025
@eread eread requested a review from thomas-zahner October 28, 2025 23:23
@nobkd
Copy link

nobkd commented Oct 28, 2025

Normally when I see . in a regex, I would expect it to match anything. Is this applicable here, and has to be escaped to be a real dot (like \.)?
(just, if this is applicable here, the regex dot would probably have coincidentally matched the real dot beforehand)

(edit: It doesn't really matter that much, as the match is not used very often, but the group would match something like 123-1/1, not only 123.1.1, and this might not be the desired match)

@eread
Copy link
Contributor Author

eread commented Oct 29, 2025

Normally when I see . in a regex, I would expect it to match anything. Is this applicable here, and has to be escaped to be a real dot (like \.)? (just, if this is applicable here, the regex dot would probably have coincidentally matched the real dot beforehand)

(edit: It doesn't really matter that much, as the match is not used very often, but the group would match something like 123-1/1, not only 123.1.1, and this might not be the desired match)

Thanks for chiming in @nobkd. I agree that . normally acts as an operator of some kind. I didn't dig in to what sort of regex parser that the action uses, just looked at the documentation: https://github.com/docker/metadata-action?tab=readme-ov-file#typesemver.

@nobkd
Copy link

nobkd commented Oct 29, 2025

They use the unescaped version of the dots in many patterns, so this probably works as intended 🤷

@thomas-zahner
Copy link
Member

True, for correctness we could replace . with \.. However, it shouldn't pose a problem as we only create tags with version numbers in the format of lychee-v1.2.3.

@thomas-zahner thomas-zahner merged commit 8096a56 into lycheeverse:master Oct 29, 2025
6 checks passed
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.

3 participants