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: support non-tag push event trigger #77

Closed
wants to merge 3 commits into from

Conversation

haya14busa
Copy link

It supports usecases that users manually tag new version with GitHub
Actions workflow, and then release it.

e.g. https://github.com/actions/create-release support tag input and
can be used for non tag push event.

@haya14busa haya14busa changed the title Add option to skip --snapshot check feat: Add option to skip --snapshot check Jan 20, 2020
@haya14busa
Copy link
Author

friendly ping? @crazy-max

@caarlos0
Copy link
Member

caarlos0 commented Feb 4, 2020

I would maybe do the inverse: a config snapshots or something like that to run goreleaser --snapshot on non-tags... 🤔

wdyt?

@crazy-max
Copy link
Member

crazy-max commented Feb 4, 2020

@haya14busa I agree with @caarlos0, adding this argument would be confusing for the end user and would overlap the args one. A dedicated workflow on non-tags could fit your needs here.

@haya14busa
Copy link
Author

Thanks. I actually totally agreed with it. This change was not to break backward compatibility.

If it's ok to break it, then we can just remove the code related with snapshot and let users use args to specify --snapshot for non-tag workflow as they need.

@haya14busa haya14busa force-pushed the nosnapshot branch 3 times, most recently from 75805e7 to 4ea3102 Compare February 4, 2020 16:03
It supports usecases that users manually tag new version with GitHub
Actions workflow, and then release it.

e.g. https://github.com/actions/create-release support `tag` input and
can be used for non tag push event.
Users can explicitly use `args: release --snapshot` instead.
@haya14busa haya14busa changed the title feat: Add option to skip --snapshot check feat: support non-tag push event trigger Feb 4, 2020
@haya14busa
Copy link
Author

Updated. PTAL @crazy-max @caarlos0

@crazy-max
Copy link
Member

@haya14busa I may have misspoken, but what I wanted to say is that the operation of this action is sufficient in its present state and does not, in my opinion, require the changes you wish to make via this PR.

To summarize the --snapshot flag is automagically inserted if no tag is defined in order to overcome the restriction set by GoReleaser and thus allow to build whatever the state of the repository. This is particularly useful in the case of PR for example.

So in your case if no tag is defined, the application will still be built as a SNAPSHOT.

@haya14busa
Copy link
Author

It's not sufficient, that's because I opened this PR.

For example, this workflow add new tag when merging pull-request (non-tag push event) and then release it via actions/create-release later step.

Because gorelease-action unnecessary add --snapshot for non-tag push event, even if the HEAD commit has actually valid tag, we cannot use this gorelease action for this use case for example.

@caarlos0
Copy link
Member

caarlos0 commented Feb 5, 2020

Because gorelease-action unnecessary add --snapshot for non-tag push event, even if the HEAD commit has actually valid tag, we cannot use this gorelease action for this use case for example.

goreleaser would still not run in this case because it also checks if current commit == latest tag, if it is not it will exit 1.

@haya14busa
Copy link
Author

haya14busa commented Feb 5, 2020

For example, this workflow add new tag when merging pull-request (non-tag push event)

The workflow adds a new tag to the current commit and also push the tag to github, so it should work.

@crazy-max
Copy link
Member

@haya14busa

The workflow adds a new tag to the current commit and also push the tag to github, so it should work.

Can you give us a link to a workflow that fails?

@haya14busa
Copy link
Author

haya14busa commented Feb 6, 2020

OK: https://github.com/haya14busa/offset/runs/430337153?check_suite_focus=true
NG: https://github.com/haya14busa/offset/runs/430366586?check_suite_focus=true

image

For NG workflow, even though head commit has latest tag, the actual release didn't happen because it's snapshot mode.

GitHub
get file position from offset. Contribute to haya14busa/offset development by creating an account on GitHub.
GitHub
get file position from offset. Contribute to haya14busa/offset development by creating an account on GitHub.

@crazy-max
Copy link
Member

@haya14busa Have you tried with your modifications? (replace goreleaser/goreleaser-action@v1 with haya14busa/goreleaser-action@nosnapshot).

@haya14busa
Copy link
Author

OK link above uses my branch and it worked.

@crazy-max
Copy link
Member

@haya14busa

For me this build "breaks" the purpose of GitHub Action and CI philosophy. Let me explain why. Everytime something is push/merge to master your action creates a new tag. It's not a best practice in a typical Gitflow. Tag must be created on purpose by maintainers.

This is why the goreleaser-action does not detect the tag and forces --snapshot. Tag detection is done through the GITHUB_REF environment variable for this worker.

If you still prefer to keep this workflow, we could improve the detection of a tag and no longer go through this environment variable. I'll think about this possibility, which I think is the most feasible.

@caarlos0
Copy link
Member

I agree with @crazy-max, creating a tag on every push to master is... well, weird...

IMHO @haya14busa can also use their own fork, but I'm not sure I'm up adding this to the action itself.

crazy-max added a commit that referenced this pull request Feb 10, 2020
Only handle snapshot flag for release cmd (#94)
Use core.info instead of console.log
@haya14busa
Copy link
Author

creating a tag on every push to master is... well, weird...

The example workflow does NOT create a tag on every push to master, but the workflow will create a tag only when merging pull-request with specific tags (e.g. bump:minor) which can be attached on purpose by maintainers.

crazy-max added a commit that referenced this pull request Feb 11, 2020
* Improve git tag detection (#77)
* Only handle snapshot flag for release cmd (#94)
* Use core.info instead of console.log
* Update gitattributes
@crazy-max
Copy link
Member

@haya14busa #95 should fix your issue.

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.

None yet

3 participants