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

feat: set 7 and latest as 7.2 aliases #16

Merged
merged 1 commit into from
May 9, 2024
Merged

feat: set 7 and latest as 7.2 aliases #16

merged 1 commit into from
May 9, 2024

Conversation

iamsauravsharma
Copy link
Contributor

Create aliases for 7.2

@madolson madolson requested review from hpatro and roshkhatri May 6, 2024 20:20
@madolson
Copy link
Member

madolson commented May 6, 2024

@roshkhatri @hpatro Can you guys take a look at this?

Copy link
Member

@roshkhatri roshkhatri left a comment

Choose a reason for hiding this comment

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

This will set the latest tag for the Valkey 7.2.5 image.

@roshkhatri roshkhatri self-requested a review May 8, 2024 19:01
@roshkhatri
Copy link
Member

roshkhatri commented May 8, 2024

On another thought and discussion with @hpatro we should probably just set the 7.2 as latest and not include 7 tag as someone might just want 7 to be as 7.0 and not bump up to 7.2. Not that we have 7.0 but just for an example.

@madolson
Copy link
Member

madolson commented May 8, 2024

@roshkhatri With semantic versioning, if you specific 7, you would expect the latest version of 7.x.y according to semantic versioning. Which in this case would be 7.2, so I think we should release a 7.

@hpatro
Copy link
Contributor

hpatro commented May 8, 2024

@roshkhatri With semantic versioning, if you specific 7, you would expect the latest version of 7.x.y according to semantic versioning. Which in this case would be 7.2, so I think we should release a 7.

Does this get called out in SemVer? From what I read here https://semver.org/#backusnaur-form-grammar-for-valid-semver-versions is it to have major.minor.patch to be a valid semver.

@madolson
Copy link
Member

madolson commented May 8, 2024

That's not exactly what I meant, I was saying that if you put in 7, I would expect it to be most recent version of 7 (ordered by semantic versioning. Where 7.2.5 is later than 7.0.99)

@iamsauravsharma
Copy link
Contributor Author

iamsauravsharma commented May 9, 2024

@roshkhatri @hpatro I think your suggestion will cause more confusion than actually helping. Lots of containers, even Redis, use X to represent X.Y.Z or X.Y which is the latest release of X. I don't think there is many widely used container that follows your suggestion, so there will be more confusion
Since it is de facto standard so as a user I would expect 7 will pull recent version of 7 instead of 7.0.X

@hpatro
Copy link
Contributor

hpatro commented May 9, 2024

Did take a look at some other popular docker containers

Bitnami Redis (https://hub.docker.com/r/bitnami/redis/tags) uses major.minor and major.minor.patch version releases and doesn't have a major version only tag.

However, NodeJS/Postgres/OpenSearch/Alpine, all support a major version only tag. With these many data point(s) I guess it's a standard practice.

@madolson madolson merged commit 2dfed05 into valkey-io:mainline May 9, 2024
8 checks passed
@iamsauravsharma iamsauravsharma deleted the update-aliases branch May 10, 2024 02:55
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.

4 participants