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

Update Goreleaser #18

Merged
merged 7 commits into from Mar 28, 2024
Merged

Update Goreleaser #18

merged 7 commits into from Mar 28, 2024

Conversation

m-timmermann
Copy link
Contributor

This PR updates the goreleaser.
It also removes deprecated keys

Comment on lines -30 to -35
replacements:
darwin: Darwin
linux: Linux
windows: Windows
386: i386
amd64: x86_64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this part.
I do not know if this part is needed or not.
see https://goreleaser.com/deprecations/#archivesreplacements for how it can be reimplemented

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine. The section was never really necessary in the first place, since the values listed here were actually part of the default anyway. The new default will also cover what we need:

https://github.com/goreleaser/goreleaser/blob/2498ea70290792c2537d98dbbdde0dd5ec07c2d6/internal/pipe/archive/archive.go#L75-L78

which yields:

{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}{{ with .Arm }}v{{ . }}{{ end }}{{ with .Mips }}_{{ . }}{{ end }}{{ if not (eq .Amd64 "v1") }}{{ .Amd64 }}{{ end }}

vs current:

'{{ .ProjectName }}_{{ .Os }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}'

I would say keep it as is or just remove the template and use the default.

@m-timmermann m-timmermann marked this pull request as ready for review March 27, 2024 07:21
Comment on lines -30 to -35
replacements:
darwin: Darwin
linux: Linux
windows: Windows
386: i386
amd64: x86_64
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine. The section was never really necessary in the first place, since the values listed here were actually part of the default anyway. The new default will also cover what we need:

https://github.com/goreleaser/goreleaser/blob/2498ea70290792c2537d98dbbdde0dd5ec07c2d6/internal/pipe/archive/archive.go#L75-L78

which yields:

{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}{{ with .Arm }}v{{ . }}{{ end }}{{ with .Mips }}_{{ . }}{{ end }}{{ if not (eq .Amd64 "v1") }}{{ .Amd64 }}{{ end }}

vs current:

'{{ .ProjectName }}_{{ .Os }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}'

I would say keep it as is or just remove the template and use the default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the yaml so off in this? Usually you do:

- name: "Checkout"
  uses: actions/checkout@v4

but here it is:

-
  name: Checkout
  uses: actions/checkout@v2

also now that I see it: the version can be bumped on the actions.

* fixed the format
* updated versions
* check version
* update version
@m-timmermann
Copy link
Contributor Author

m-timmermann commented Mar 27, 2024

Fixed the formatting.
Updated the version for go, actions/checkout@v4 and actions/setup-go@v5

The moment i update goreleaser/goreleaser-action@v1 the workflow wants me to add a tag or specify ---snpashot in the arguments for the goreleaser
⨯ release failed after 0s error=git doesn't contain any tags. Either add a tag or use --snapshot

see https://github.com/meplato/store2-go-client/actions/runs/8451327839/job/23149434753#step:4:23

@rwenz3l
Copy link
Contributor

rwenz3l commented Mar 27, 2024

@m-timmermann the fetch-depth of the checkout action is 1 by default, so unless your latest commit contains a tag, it will rightfully complain. Usually you trigger this workflow on tags only, and in that case it should work. If you want to ensure there is a tag to build upon, use fetch-depth=0.

Comment on lines +11 to +12
- name: Checkout
uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add fetch-depth: 0 if you are unsure about tags being present. In the default case (when you release on tags, there will be a tag associated to the ref being checked out by git. You can test this by creating a dummy-tag on your latest commit in this PR.

Copy link
Member

@XoseRamon XoseRamon left a comment

Choose a reason for hiding this comment

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

check the version of go-releaser. Otherwise, this LGTM

-
name: Run GoReleaser
go-version: 1.21
- name: Run GoReleaser
uses: goreleaser/goreleaser-action@v1
Copy link
Member

Choose a reason for hiding this comment

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

In think go releaser is at version 5 or something like that already

go-version: 1.18.x
-
name: Run GoReleaser
go-version: 1.21
Copy link
Member

Choose a reason for hiding this comment

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

You can go with 1.22, since we even have a .1 release

* update goreleaser
* remove workflow on pr
* update go version
@m-timmermann
Copy link
Contributor Author

m-timmermann commented Mar 28, 2024

Updated goreleaser version
Updated go version

The release workflow will now only be triggered if a tag was pushed.
Tested the goreleaser. It only works with tags which have a Valid Semantic Version e.g. v2.1.14

* only trigger for v* tags
@m-timmermann m-timmermann merged commit 1318e70 into main Mar 28, 2024
6 checks passed
@m-timmermann m-timmermann deleted the update-goreleaser branch March 28, 2024 08:17
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