-
Notifications
You must be signed in to change notification settings - Fork 28
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
Goreleaser package management #47
Conversation
@HeavyWombat do you have any experience with RPM/deb creations using nFPM => https://github.com/goreleaser/nfpm ? @otaviof do you think that having also RPMs would be interesting for us? That would be for upstream only by the way. |
/assign @HeavyWombat |
@akram I'm okay if we look at more advanced packing options, like RPMs, as a next step. So, we can review this pull-request as it's and plan RPMs later on :-) |
Yes, that would be nice. Especially that nFPM seems to be a totally different product and I didn't see any integration with goreleaser. |
/approve @akram if you still want feedback from @HeavyWombat on his experiences re: #47 (comment), and if he does not have cycles to catch our tagging of him here, see about pinging him in the shipwright channel on k8s slack fwiw I am fine merging this as an initial step as is, with follow ups with @HeavyWombat as well as what you and @otaviof already discussed |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gabemontero The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I am sorry for the delayed response. Was booked with so many other things. With regards to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I would opt for removing the go mod tidy
just to keep the hooks small and focused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We just realized during our Shipwright community, we should also use this PR to include a release GitHub Action. One example would be: https://github.com/homeport/dyff/blob/main/.github/workflows/release.yml. This works for Homebrew and Snapcraft.
This pull request shows 19 extra commits, I think something went awry with your rebase ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some other things beside the problem that somehow other commits landed in this PR.
@akram it looks like a rebase broke the commit history here. Can you please rebase this branch so only the commits we're interested in are included? |
@adambkaplan and @coreydaley indeed, I don't know what happened; my git client was updated and some default configuration changed. Now it is fixed; I synced my main branch and cherry-picked the right commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @adambkaplan may you check the secret references from Akram in the repository settings?
/assign adambkaplan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting up credentials for Homebrew and Snap will require additional effort. Can this be scaled back so that we just upload cross-compiled binaries to Github?
args: release --rm-dist | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
HOMEBREW_TAP_GITHUB_TOKEN: ${{ secrets.HOMEBREW_TAP_GITHUB_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akram is this something we need to add to the organization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's it.
sudo snap install snapcraft --classic | ||
snapcraft login --with - <<<"${SNAP_TOKEN}" | ||
env: | ||
SNAP_TOKEN: ${{ secrets.SNAP_LOGIN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akram looks like this also needs to be added to the organization (we should coordinate offline)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
/close In favor of #61 |
@adambkaplan: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Changes
Enables Goreleaser to allow pacjage releases.
Fixes #23
Submitter Checklist
Release Notes