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

Add release process #89

Merged
merged 20 commits into from
Jul 23, 2020
Merged

Add release process #89

merged 20 commits into from
Jul 23, 2020

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Jul 20, 2020 •
edited
Loading

Adds a release process:

  • Run acceptance tests on merges to master against snapshot artifacts (also make snapshot make target). Acceptance tests involve running the installed binaries against known good output.
  • Add release pipeline that publishes release artifacts on git tag pushed to master (also make release make target). Note: this only continues when all static analysis, unit, integration, and acceptance tests have already run for the given tagged commit SHA.
  • Adds publishing to a homebrew tap (anchore/homebrew-imgbom)

This PR also achieves the following:

  • Removes dependency on CGO (replaces go-rpmdb with a fork)
  • Moves all forms of tests (except unit) under the /test dir

Todo:

  • The final release process skips all publishing for the meantime, we will turn this on closer to release and after renaming the tool.
  • Uploading the VERSION file to an S3 bucket will be completed in another ticket

@wagoodman wagoodman added this to the v0.1.0 milestone Jul 20, 2020
@wagoodman wagoodman self-assigned this Jul 20, 2020
@zhill
Copy link
Member

zhill commented Jul 20, 2020

@wagoodman I think that list looks good

Base automatically changed from add-update-check to master July 21, 2020 16:02
@wagoodman wagoodman force-pushed the add-release-pipeline branch 5 times, most recently from 5a34666 to 7030a83 Compare July 21, 2020 18:26
@wagoodman wagoodman force-pushed the add-release-pipeline branch 11 times, most recently from 69e917a to 21ffdc5 Compare July 21, 2020 21:09
@wagoodman wagoodman marked this pull request as ready for review July 22, 2020 14:37
@wagoodman wagoodman mentioned this pull request Jul 22, 2020
Closed
alfredodeza
alfredodeza reviewed Jul 22, 2020
View reviewed changes
.circleci/config.yml Show resolved Hide resolved
.github/workflows/acceptance-test.yaml Outdated
# act on a newly opened pull request (supports forks)
pull_request:
push:
# act on a push to any branch
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about this one, are you envisioning that we will rely more on pushes to dev branches for testing, before opening a PR? Or is there a different scenario for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can make this one less aggressive, say only pushes to master? the downside is that it would be tricky to see an acceptance test issue before merging a PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

oooh I thought this was aside from PRs, but you are saying this would be triggered only after a merge. Let's keep it then

Copy link
Contributor Author

@wagoodman wagoodman Jul 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

sorry, bad phrasing on my part: your interpretation was correct, this will trigger on a push to any branch. I was suggesting that I could make a change to make acceptance tests only run when there is a merge to master specifically (making it less aggressive of a check).

I'm a bit torn either way --acceptance tests on PRs (on each push) is a waste of CPU cycles and time... while acceptance tests only for master commits seems like we could miss problems that come up on PRs (and break master).

I'll make the adjustment for now to make this less aggressive and only run acceptance tests on pushes to master only.

.github/workflows/acceptance-test.yaml Show resolved Hide resolved
cmd/root.go Show resolved Hide resolved
test/acceptance/deb.sh Outdated Show resolved Hide resolved
test/acceptance/mac.sh Outdated Show resolved Hide resolved
@alfredodeza
Copy link
Contributor

There are a couple of merge commits that made it into this PR's history. I'd rather have clean commits without the merges

@wagoodman
Copy link
Contributor Author

There are a couple of merge commits that made it into this PR's history. I'd rather have clean commits without the merges

I'll be squash merging

alfredodeza
alfredodeza approved these changes Jul 23, 2020
View reviewed changes
Copy link
Contributor

@alfredodeza alfredodeza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks great. My only comment is having merge commits which don't look necessary at all here and could be cleaned up with a rebase . It turns out you can squash+merge PRs!

zhill
zhill reviewed Jul 23, 2020
View reviewed changes
imgbom/cataloger/rpmdb/parse_rpmdb.go
@@ -9,7 +9,7 @@ import (
"github.com/anchore/imgbom/imgbom/pkg"
"github.com/anchore/imgbom/internal"
"github.com/anchore/imgbom/internal/log"
rpmdb "github.com/knqyf263/go-rpmdb/pkg"
rpmdb "github.com/wagoodman/go-rpmdb/pkg"
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this into anchore soon? For same reasons: ensuring the team can update/manage as needed and not gated specifically on you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to the todo list 👍

zhill
zhill approved these changes Jul 23, 2020
View reviewed changes
Copy link
Member

@zhill zhill left a comment

Choose a reason for hiding this comment

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

lgtm. One comment about the go-rpm fork landing spot, but that isn't a blocking issue.

@wagoodman wagoodman merged commit ba4f630 into master Jul 23, 2020
@wagoodman wagoodman deleted the add-release-pipeline branch July 23, 2020 14:53
@wagoodman wagoodman mentioned this pull request Jul 27, 2020
Closed
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
@wagoodman
Add release process (anchore#89)
9746734 
* add check for app update; fix ETUI error handling

* validate user args

* add goreleaser support

* replace cgo dependencies (go-rpm) with go equivalents

* add acceptance tests against build snapshot

* add brew tap + acceptance test pipeline

* add mac acceptance tests

* fix compare makefile

* fix mac acceptance tests

* add release pipeline with wait checks

* add token to release step

* rm dir presenters int test

* enforce dpkg to be non interactive

Co-authored-by: Alfredo Deza <adeza@anchore.com>

* pin brew formulae

* pin skopeo to formulae url

* only run acceptance tests

Co-authored-by: Alfredo Deza <adeza@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alfredodeza alfredodeza alfredodeza approved these changes

@Btodhunter Btodhunter Btodhunter approved these changes

@zhill zhill zhill approved these changes

@robertp robertp Awaiting requested review from robertp

Assignees

@wagoodman wagoodman

Labels
None yet
Projects
None yet
Milestone
Syft v0.1.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wagoodman @zhill @alfredodeza @Btodhunter