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

sso: update go modules #273

Merged
merged 3 commits into from
Dec 9, 2019
Merged

Conversation

Jusshersmith
Copy link
Contributor

Problem

Updates to certain go modules are required. This is starting to cause errors with new builds: https://circleci.com/gh/buzzfeed/sso/2185?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Solution

Running https://github.com/buzzfeed/sso/blob/master/scripts/test locally edited go.mod and go.sum files as required.

diff created by running scripts/test
@mccutchen
Copy link
Contributor

Should we also add -mod=readonly to the go build commands in our Dockerfile and Makefile to more explicitly require up-to-date module information?

@Jusshersmith
Copy link
Contributor Author

Yeah, I think that sounds like a sensible idea @mccutchen!

@codecov
Copy link

codecov bot commented Dec 6, 2019

Codecov Report

Merging #273 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #273   +/-   ##
=======================================
  Coverage   62.21%   62.21%           
=======================================
  Files          54       54           
  Lines        4200     4200           
=======================================
  Hits         2613     2613           
  Misses       1399     1399           
  Partials      188      188

mccutchen
mccutchen previously approved these changes Dec 6, 2019
Copy link
Contributor

@mccutchen mccutchen left a comment

Choose a reason for hiding this comment

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

LGTM aside from one small nit, thanks @Jusshersmith!

Dockerfile Outdated Show resolved Hide resolved
@Jusshersmith
Copy link
Contributor Author

Think I'm going to need a fresh review on this @mccutchen! (thanks!)

Copy link
Contributor

@mccutchen mccutchen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Jusshersmith!

@Jusshersmith Jusshersmith merged commit e49ca7a into master Dec 9, 2019
@Jusshersmith Jusshersmith deleted the jusshersmith-update-go-modules branch December 9, 2019 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants