Skip to content

Docker: Handle versions with underscore characters#4801

Closed
RodrigoPetter wants to merge 1 commit intodependabot:mainfrom
RodrigoPetter:main
Closed

Docker: Handle versions with underscore characters#4801
RodrigoPetter wants to merge 1 commit intodependabot:mainfrom
RodrigoPetter:main

Conversation

@RodrigoPetter
Copy link
Copy Markdown
Contributor

Allows the underscore character to be present in the prefix, root or
suffix.

Also fixes a bug where numbers were not allowed to be present in the
prefix or in the suffix when prefix and suffix were present at the
same time.

Allows the underscore character to be present in the prefix, root or
suffix.

Also fixes a bug where numbers were not allowed to be present in the
prefix or in the suffix when prefix and suffix were present at the
same time.
@RodrigoPetter RodrigoPetter requested a review from a team as a code owner March 4, 2022 19:08
return unless tag.match?(NAME_WITH_VERSION)

tag.match(NAME_WITH_VERSION).named_captures.fetch("version").downcase
tag.match(NAME_WITH_VERSION).named_captures.fetch("version").downcase.gsub(/_/i, ".")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably a better place to handle this change would be in the version.rb file, as has been done for other languages like Gradle, but I wouldn't know how to do that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @RodrigoPetter! I didn't see your PR in time and created a separate one at #5734. Coincidentally, I used the approach you mentioned here used for other ecosystems 👍.

Anyways, this PR was very useful and I reused part of the specs and added attribution to you in the commit ❤️.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wow @deivid-rodriguez, thank you! I'm glad I could help.

Probably your PR fixes the original problem that I was facing. My PR doesn't seems necessary anymore.

I would prefer that, for more complex cases of non semver, it would be taken into consideration the cases in the issues #2692 and #4329.

With that in mind, I'm closing my PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you! We'll try to keep improving things and you're welcome to keep sending us patches. We'll try to be more responsive next time!

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.

2 participants