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.

Sign up for GitHub

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

Jump to bottom

Goreleaser package management #47

Closed
wants to merge 2 commits into from
Closed

Goreleaser package management #47

wants to merge 2 commits into from

Conversation

akram
Copy link
Contributor

@akram akram commented Sep 28, 2021 •
edited by HeavyWombat
Loading

Changes

Enables Goreleaser to allow pacjage releases.

Fixes #23

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

Release Notes

Adds Goreleaser to allow package releases.

@openshift-ci openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 28, 2021
@akram
Copy link
Contributor Author

akram commented Sep 29, 2021

@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.

@akram akram changed the title WIP: Goreleaser package management Goreleaser package management Sep 29, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 29, 2021
@akram
Copy link
Contributor Author

akram commented Sep 29, 2021

/assign @HeavyWombat

@otaviof
Copy link
Member

otaviof commented Sep 29, 2021 •
edited
Loading

@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 :-)

@akram
Copy link
Contributor Author

akram commented Sep 29, 2021

Yes, that would be nice. Especially that nFPM seems to be a totally different product and I didn't see any integration with goreleaser.

@gabemontero
Copy link
Member

/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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 5, 2021

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2021
@adambkaplan adambkaplan added this to the release-v0.6.0 milestone Oct 6, 2021
@HeavyWombat
Copy link
Contributor

@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.

I am sorry for the delayed response. Was booked with so many other things.

With regards to nFPM, I have no experience with this. So far, I only used GoReleaser for Homebrew Taps and Snap where only the raw binaries do the trick.

HeavyWombat
HeavyWombat requested changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@HeavyWombat HeavyWombat left a 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.

.goreleaser.yml Outdated Show resolved Hide resolved
@HeavyWombat HeavyWombat added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 11, 2021
@SaschaSchwarze0 SaschaSchwarze0 mentioned this pull request Oct 11, 2021
Closed
HeavyWombat
HeavyWombat requested changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@HeavyWombat HeavyWombat left a 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.

@coreydaley
Copy link
Contributor

coreydaley commented Oct 11, 2021 •
edited
Loading

This pull request shows 19 extra commits, I think something went awry with your rebase ...

SaschaSchwarze0
SaschaSchwarze0 requested changes Oct 11, 2021
View reviewed changes
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a 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.

.github/workflows/release.yml Show resolved Hide resolved
@adambkaplan
Copy link
Member

@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?

@akram
Copy link
Contributor Author

akram commented Oct 11, 2021 •
edited
Loading

@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.

SaschaSchwarze0
SaschaSchwarze0 approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a 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

adambkaplan
adambkaplan requested changes Oct 13, 2021
View reviewed changes
Copy link
Member

@adambkaplan adambkaplan left a 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?

.github/workflows/release.yml
args: release --rm-dist
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
HOMEBREW_TAP_GITHUB_TOKEN: ${{ secrets.HOMEBREW_TAP_GITHUB_TOKEN }}
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's it.

.github/workflows/release.yml
sudo snap install snapcraft --classic
snapcraft login --with - <<<"${SNAP_TOKEN}"
env:
SNAP_TOKEN: ${{ secrets.SNAP_LOGIN }}
Copy link
Member

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

@imjasonh imjasonh mentioned this pull request Oct 18, 2021
Closed
@adambkaplan
Copy link
Member

/close

In favor of #61

@openshift-ci openshift-ci bot closed this Oct 25, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 25, 2021

@adambkaplan: Closed this PR.

In response to this:

/close

In favor of #61

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SaschaSchwarze0 SaschaSchwarze0 SaschaSchwarze0 approved these changes

@HeavyWombat HeavyWombat HeavyWombat requested changes

@adambkaplan adambkaplan adambkaplan requested changes

@imjasonh imjasonh Awaiting requested review from imjasonh

@sbose78 sbose78 Awaiting requested review from sbose78

Assignees

@adambkaplan adambkaplan

@HeavyWombat HeavyWombat

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. release-note
Projects
None yet
Milestone
release-v0.6.0
Development

Successfully merging this pull request may close these issues.

Add GoReleaser with Homebrew/Snap releases
7 participants
@akram @otaviof @gabemontero @HeavyWombat @coreydaley @adambkaplan @SaschaSchwarze0