-
Notifications
You must be signed in to change notification settings - Fork 74
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 #80
GoReleaser #80
Conversation
docker: | ||
- image: circleci/golang:1.13 | ||
environment: | ||
GO111MODULE: "on" |
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.
Is GO111MODULE: "on"
still necessary for go 1.13?
The GO111MODULE environment variable continues to default to auto, but the auto setting now activates the module-aware mode of the go command whenever the current working directory contains, or is below a directory containing, a go.mod file — even if the current directory is within GOPATH/src. This change simplifies the migration of existing code within GOPATH/src and the ongoing maintenance of module-aware packages alongside non-module-aware importers.
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.
I think I'm gonna leave it for now, but with Go 1.14 I don't mind removing it?
.circleci/config.yml
Outdated
path: /tmp/test-results | ||
destination: raw-test-output | ||
- store_test_results: | ||
path: /tmp/test-results |
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.
can/should you use TEST_RESULTS
here?
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.
I forgot why I did this, just kinda muscle memory at this point. CircleCI does not support an environment variable here, for some whatever reason!
algorithm: sha256 | ||
|
||
snapshot: | ||
name_template: "{{ .Tag }}-next-{{.FullCommit}}" |
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.
Does this pull in the previous tag value?
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.
It pulls in the current tag value. The way GoReleaser works, is someone creates a tag, then CircleCI would start a build and release it. I preferred this a tiny bit to what we had before because it makes doing major and minor releases possible. Goreleaser also has a ton of functionality related to Docker, rpms, etc... that we could leverage.
README.md
Outdated
@@ -5,6 +5,8 @@ | |||
<a href="https://travis-ci.org/sonatype-nexus-community/nancy"><img src="https://travis-ci.org/sonatype-nexus-community/nancy.svg?branch=master" alt="Build Status"></img></a> |
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 should also cleanup travisci too.
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.
Done!
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.
I should have been clearer here..... :) we should also delete the .travis.yml as well :) No need to support both CI's.
ldflags: | ||
- -s -w -X "github.com/sonatype-nexus-community/nancy/buildversion.BuildVersion={{.Version}}" | ||
- -s -w -X "github.com/sonatype-nexus-community/nancy/buildversion.BuildTime={{time "2006-01-02T15:04:05Z07:00"}}" | ||
- -s -w -X "github.com/sonatype-nexus-community/nancy/buildversion.BuildCommit={{.FullCommit}}" |
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.
The default value is a one liner, https://goreleaser.com/customization/#Builds
- -s -w -X "github.com/sonatype-nexus-community/nancy/buildversion.BuildCommit={{.FullCommit}}" | |
- >- | |
-s -w | |
-X "github.com/sonatype-nexus-community/nancy/buildversion.BuildVersion={{.Version}}" | |
-X "github.com/sonatype-nexus-community/nancy/buildversion.BuildTime={{time "2006-01-02T15:04:05Z07:00"}}" | |
-X "github.com/sonatype-nexus-community/nancy/buildversion.BuildCommit={{.FullCommit}}" |
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.
I'm not ANTI this, but I kinda like it as three separate commands (since it's doing it three different places). I don't have enough experience with goreleaser to know if one is preferable to the other. My main concern would be if one fails, etc...
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.
The readability is I THINK easier with it broken out like this, is my main pro to having them the way I have it.
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.
But yours isn't bad for that either (it's just clear on the original that it's running -s -w -X on all three, at a quick glance)
Co-Authored-By: Yoan Blanc <yoan@dosimple.ch>
@zendern here's an example with a failing test: https://app.circleci.com/jobs/github/sonatype-nexus-community/nancy/19 Also you can see the old experience here: https://circleci.com/gh/sonatype-nexus-community/nancy/19#tests/containers/0 Those steps I believe run no matter what to store artifacts/test results, so we should get the info all the time! You can see the details from the test failure in that tab. An alternative would be to run |
@zendern @fitzoh @greut : we tested the goreleaser release on tag stuff on @allenhsieh 's fork: https://github.com/allenhsieh/nancy/releases/tag/v0.1.1 https://circleci.com/gh/allenhsieh/nancy/6 The only thing I noticed that I'm like EHHHH on is it's creating zips/tar.gz's so it adds a step to someone's download process. I think that's OK, but I wanted to run it past a few other people. |
@DarthHater Looks good to me. We still need to update the README.md regarding install with tars being involved now and if I understand how it will work now it will no longer be on merge into master. It'll be when a tag is created so we should update that. I do feel like we should probably bump it to 1.0.0. With this and #59 technically changing a public method signature (not sure if anyone is using it in that way) simply just changing to a minor version doesn't feel like enough since this is a breaking change there and in packaging. |
@zendern @fitzoh @greut , we got it ALL figured out, and @allenhsieh just pushed up the changes to make it happen. If you'd like to approve/rubber stamp, now is a good time :) |
I did!
…On Wed, Feb 26, 2020 at 4:52 PM Nathan Zender ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In README.md
<#80 (comment)>
:
> @@ -5,6 +5,8 @@
<a href="https://travis-ci.org/sonatype-nexus-community/nancy"><img src="https://travis-ci.org/sonatype-nexus-community/nancy.svg?branch=master" alt="Build Status"></img></a>
I should have been clearer here..... :) we should also delete the
.travis.yml as well :) No need to support both CI's.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#80?email_source=notifications&email_token=ABKJTBTIM5OWGNXGDGTCWE3RE4MHLA5CNFSM4KZU32JKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCXDRCLA#discussion_r384872153>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKJTBRM6BVIVMUQQ5VF6R3RE4MHLANCNFSM4KZU32JA>
.
|
Current changelist looks like so :) maybe you forgot to push it?? |
@DarthHater do we also need this anymore?? https://github.com/sonatype-nexus-community/nancy/blob/master/bumpver.sh |
And |
@nrcook is typically a genius, this is a freshened up version of what he did in #20 .
This pull request makes the following changes:
This should support Alpine Linux as we disable CGO, so all current cases should be covered.
We were able to 1:1 it with the current Nancy releases, and tested it out on @allenhsieh fork of Nancy:
https://github.com/allenhsieh/nancy/releases/tag/v0.1.7
We added a few builds as well, while doing that, mostly for 32 bit use.
cc @bhamail / @DarthHater / @zendern / @fitzoh