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

Experiment with Go Modules #1285

Merged
merged 15 commits into from May 31, 2019

Conversation

Projects
None yet
3 participants
@gdbelvin
Copy link
Collaborator

commented May 24, 2019

No description provided.

@gdbelvin gdbelvin requested a review from google/keytransparency as a code owner May 24, 2019

@googlebot googlebot added the cla: yes label May 24, 2019

@gdbelvin gdbelvin force-pushed the gdbelvin:modules branch 2 times, most recently from 2f7093f to 78d10d2 May 24, 2019

@codecov

This comment has been minimized.

Copy link

commented May 24, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1285   +/-   ##
=======================================
  Coverage   31.95%   31.95%           
=======================================
  Files          45       45           
  Lines        3655     3655           
=======================================
  Hits         1168     1168           
  Misses       2307     2307           
  Partials      180      180

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8c35f9...c8efecb. Read the comment docs.

@gdbelvin gdbelvin force-pushed the gdbelvin:modules branch from cb59eb6 to 4f2c8ed May 24, 2019

@gdbelvin gdbelvin force-pushed the gdbelvin:modules branch from 4f2c8ed to 9f1fce9 May 24, 2019

@gdbelvin gdbelvin force-pushed the gdbelvin:modules branch from a2dfc09 to 1422719 May 24, 2019

@gdbelvin gdbelvin requested a review from RJPercival May 29, 2019

@RJPercival
Copy link
Member

left a comment

If you stick with this, then you may want to update your README.md to instruct people to take advantage of it (right now, that says to use go get).


script:
- go build ./...
- golangci-lint run
- golangci-lint run --deadline=5m

This comment has been minimized.

Copy link
@RJPercival

RJPercival May 29, 2019

Member

Is the timeout necessary both here and in .golangci.yml?

This comment has been minimized.

Copy link
@gdbelvin

gdbelvin May 31, 2019

Author Collaborator

Testing showed that the command didn't pick up the one in golangci.yml

This comment has been minimized.

Copy link
@RJPercival

RJPercival May 31, 2019

Member

Strange. What effect does the setting in golangci.yml have? If none, we should probably remove it.

github.com/golang/mock v1.3.1
github.com/golang/protobuf v1.3.1
github.com/google/btree v1.0.0 // indirect
github.com/google/certificate-transparency-go v1.0.21 // indirect

This comment has been minimized.

Copy link
@RJPercival

RJPercival May 29, 2019

Member

This is a fairly old release of certificate-transparency-go (2018-08-20) compared to the version of Trillian being used (from 2019-05-24). You're certain this won't cause any problems?

This comment has been minimized.

Copy link
@gdbelvin

gdbelvin May 31, 2019

Author Collaborator

It is old... but it is also the latest tag. Should we create a new release of ct-go?

This comment has been minimized.

Copy link
@gdbelvin

gdbelvin May 31, 2019

Author Collaborator

The tests pass. Are there fixed issues that are internally consistent but would cause issues for others?

This comment has been minimized.

Copy link
@RJPercival

RJPercival May 31, 2019

Member

It's most likely fine, but this combination of versions is unlikely to have been tested beyond the unit/integration tests (compared to picking commits/releases that are closer together temporally, which will be more similar to versions that went through CI and into production together).

@gdbelvin gdbelvin merged commit 1d8b1b4 into google:master May 31, 2019

5 checks passed

GolangCI No issues found!
Details
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed
codecov/patch Coverage not affected when comparing d8c35f9...c8efecb
Details
codecov/project 31.95% remains the same compared to d8c35f9
Details

@gdbelvin gdbelvin deleted the gdbelvin:modules branch May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.