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

27 Fix goconvey build failure #28

Merged
merged 2 commits into from
Apr 30, 2020
Merged

27 Fix goconvey build failure #28

merged 2 commits into from
Apr 30, 2020

Conversation

sgnn7
Copy link
Contributor

@sgnn7 sgnn7 commented Apr 30, 2020

To support the newest version of goconvey that doesn't break our build,
we needed to update our Dockerfile builds to Golang v1.13. Small cleanup of
gomod/godep files was done as part of the change.

Fixes #27

@sgnn7 sgnn7 marked this pull request as ready for review April 30, 2020 18:49
To support the newest version of goconvey that doesn't break our build,
we needed to update our Dockerfile builds to Golang v1.13.

Upstream: smartystreets/goconvey#600
Since the container goreleaser version changed, we needed to update the
local version too. In the process:
- Module metadata was cleaned up
- Remnants of godep were removed (automatically)
Copy link
Contributor

@izgeri izgeri left a comment

Choose a reason for hiding this comment

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

lgtm but not sure if you want to revise the changelog msg

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Changed
- Upgraded build/test images to use Golang 1.13 instead of 1.11 ([#27](https://github.com/cyberark/summon-aws-secrets/issues/27))
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a user facing change? is there some value to the user that we should be calling out here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I added it intentionally because this is possibly a user-visible change because the binaries generated for releases may not behave 1:1 as expected between these versions. I can remove this if needed.

@izgeri izgeri merged commit 256bb19 into master Apr 30, 2020
@izgeri izgeri deleted the 27-fix-goconvey-failure branch April 30, 2020 21:24
@izgeri
Copy link
Contributor

izgeri commented Apr 30, 2020

@sgnn7 we might have to revise the changelog message later, FYI. I think it should probably say something at the end like "Please note that since this upgrades the version of golang that we use to build the binaries to 1.13, the binaries generated for releases may not be exactly as before."

but really, we should have tests to capture that so we have some assurance that this project isn't impacted by golang version bumps.

@sgnn7
Copy link
Contributor Author

sgnn7 commented Apr 30, 2020

Fair enough - I'll keep that in mind for future work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix build failures due to goconvey updates
2 participants