Skip to content
This repository has been archived by the owner on Jun 22, 2021. It is now read-only.

feat: tag with latest #20

Closed
wants to merge 2 commits into from
Closed

Conversation

felipecrs
Copy link

@felipecrs felipecrs commented Jun 19, 2020

This new input variable is a boolean, however, it accepts 3 different values:
false, true, or not present. If present it will always take precedence over
the tag with ref input variable.

Closes docker/build-push-action#40 and closes docker/build-push-action#43.

This new input variable is a boolean, however it accepts 3 different values:
false, true or not present. If present it will always take precendence over
the tag with ref input variable.

Signed-off-by: Felipe Santos <felipecassiors@gmail.com>
@felipecrs
Copy link
Author

I'm welcome to tips, dealing with nullable bool in Go is difficult.

@felipecrs felipecrs marked this pull request as draft June 19, 2020 11:57
Comment on lines 1 to 9
{
// See https://go.microsoft.com/fwlink/?LinkId=827846 to learn about workspace recommendations.
// Extension identifier format: ${publisher}.${name}. Example: vscode.csharp

// List of extensions which should be recommended for users of this workspace.
"recommendations": ["golang.go"],
// List of extensions recommended by VS Code that should not be recommended for users of this workspace.
"unwantedRecommendations": []
}

Choose a reason for hiding this comment

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

I think this file does not belong to this repository as it is related to your local development environment.

Copy link
Author

Choose a reason for hiding this comment

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

Actually this file was intended. It just tells people using VS Code that the Go Lang extension is recommended. https://code.visualstudio.com/docs/editor/extension-gallery#_workspace-recommended-extensions

Copy link
Author

Choose a reason for hiding this comment

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

I can obviously remove according to your preference, but I thought it as a welcome addition.

Choose a reason for hiding this comment

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

I am not a maintainer of this repo, so my preference does not matter as much. I just thought that a developer's local development setup does not belong to a repo unless the repo itself dictates its development environment. If VIM, Emacs, Atom, and Sublime users start to add their project-specific configs then every repository will become a mess.

@ibnesayeed
Copy link

I'm welcome to tips, dealing with nullable bool in Go is difficult.

Should we change the API from boolean to a pattern? For example, latest_tag_ref, by default set to refs/heads/master, but can be set to something like refs/tags/* or refs/heads/main?

@felipecrs
Copy link
Author

I'm welcome to tips, dealing with nullable bool in Go is difficult.

Should we change the API from boolean to a pattern? For example, latest_tag_ref, by default set to refs/heads/master, but can be set to something like refs/tags/* or refs/heads/main?

I think boolean is still better since it gives the user all the freedom. For most docker images, the latest tag is the most important one and some projects might use a custom (and complex?) condition for when to uploading the latest tag.

@ibnesayeed
Copy link

I think boolean is still better since it gives the user all the freedom. For most docker images, the latest tag is the most important one and some projects might use a custom (and complex?) condition for when to uploading the latest tag.

I think it will be a lot more intuitive for people who are coming from DockerHub automated builds setup to be able to target specific reference (tag or branch) using patterns to build as the latest tag. Those custom and complex conditions beyond refs that one might use to determine whether or not something be tagged as latest can simply be put in the if: clause of the step to skip when not needed.

However, if you insist on booleans, then I guess you can add two booleans instead of one nullable.

@felipecrs
Copy link
Author

Those custom and complex conditions beyond refs that one might use to determine whether or not something be tagged as latest can simply be put in the if: clause of the step to skip when not needed.

Is there how to conditionally pass an input variable to an Action?

@ibnesayeed
Copy link

Is there how to conditionally pass an input variable to an Action?

I am not too sure about it, but even if it would be possible, documentation will be complicated.

I still feel that making tag_with_latest a reference pattern (with default targeting master branch) instead of boolean will be simpler to implement, easier to explain, and will cover most of the use cases. If there are more peculiar cases that won't be covered with that, in the future we can introduce more parameters.

@felipecrs
Copy link
Author

felipecrs commented Jun 24, 2020

I still feel that making tag_with_latest a reference pattern (with default targeting master branch) instead of boolean will be simpler to implement, easier to explain, and will cover most of the use cases. If there are more peculiar cases that won't be covered with that, in the future we can introduce more parameters.

I think we'll need a third opinion on that. Further than only specifying something like refs/tags/*, for example, someone could want to check if the tag published is the latest one based on semver calculations. And the implementation is already done, by the way.

@ibnesayeed
Copy link

I think we'll need a third opinion on that. Further than only specifying something like refs/tags/*, for example, someone could want to check if the tag published is the latest one based on semver calculations. And the implementation is already done, by the way.

I think it will be possible to filter alpha, beta, and release candidate versions out simply by using stricter patterns. For example, refs/tags/v?[\d\.]+$ will accept 1.2.34 and v1.2.34, but not 1.2.34-beta1.

@felipecrs
Copy link
Author

I think it will be possible to filter alpha, beta, and release candidate versions out simply by using stricter patterns. For example, refs/tags/v?[\d\.]+$ will accept 1.2.34 and v1.2.34, but not 1.2.34-beta1.

How about maintenance releases? The latest version can be v2.0.0, but a new maintenance release such as v1.5.2 can be released after v2.0.0, and it should not be tagged as latest.

Signed-off-by: Felipe Santos <felipecassiors@gmail.com>
@ibnesayeed
Copy link

How about maintenance releases? The latest version can be v2.0.0, but a new maintenance release such as v1.5.2 can be released after v2.0.0, and it should not be tagged as latest.

Oh, I did not realize this scenario. You are right.

I have an idea about this, say, we have a flag tag_semver_releases, which, when set to true, will automatically create various tags. For example, when 1.2.34 is released, it can tag with 1.2.34, 1.2, and 1, if appropriate, based on SemVer resolution. Also, since we can assume access to all version tags, the code can also realize when latest tag needs to be applied.

@felipecrs
Copy link
Author

felipecrs commented Jun 24, 2020

I have an idea about this, say, we have a flag tag_semver_releases, which, when set to true, will automatically create various tags. For example, when 1.2.34 is released, it can tag with 1.2.34, 1.2, and 1, if appropriate, based on SemVer resolution. Also, since we can assume access to all version tags, the code can also realize when latest tag needs to be applied.

I support that and it deserves its own Issue. By the way, this is already supported by publish-docker: https://github.com/elgohr/Publish-Docker-Github-Action#tag_semver.

@chris-crone
Copy link
Member

Thanks for the PR but this action is now deprecated. Please take a look at the upgrade notes to move to the build-push action.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New boolean input tag_with_latest Enable use of non-master branch as "latest"
3 participants