Skip to content

Go mod tidy sourcegraph #40

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

Closed
wants to merge 1 commit into from
Closed

Conversation

eseliger
Copy link
Member

@eseliger eseliger commented Mar 2, 2020

This is a campaign run to fulfill the delivery plan https://github.com/sourcegraph/customer/issues/13.
Over the coming days, I will update more repositories using go mod tidy.

@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #40 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #40   +/-   ##
=======================================
  Coverage   26.91%   26.91%           
=======================================
  Files           5        5           
  Lines        1133     1133           
=======================================
  Hits          305      305           
  Misses        784      784           
  Partials       44       44           

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 3637c60...7a60fdd. Read the comment docs.

Copy link
Member

@keegancsmith keegancsmith left a comment

Choose a reason for hiding this comment

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

This seems to just make it so we can't build with an earlier go? So I'd rather we didn't merge this.

@eseliger
Copy link
Member Author

eseliger commented Mar 2, 2020

I have unfortunately no idea what the directive is used for :D but I'm happy to close it, as that will also be a good data point for me testing out the roll-out with all sorts of scenarios!
I might end up moving this campaign to k8s.sgdev.org so I might have to reopen this PR, will close again then though

@dmitshur
Copy link
Contributor

dmitshur commented Mar 2, 2020

Setting the Go language version to 1.14 means that language features that are new to 1.14 may be used, and things that were deprecated/removed may not be.

Since there aren't code changes here, the packages in this module would still build successfully with older versions of Go.

It'd be completely reasonable to specify an older version of the Go language if your intention is not to use any newer Go language features.

See issue golang/go#30791 about documenting the go directive better so it's easier for module authors to choose a good value for it.

@keegancsmith
Copy link
Member

Thanks @dmitshur. So this won't affect any modules which depend on go-diff? If so LGTM

@keegancsmith
Copy link
Member

Next time I will read the link before asking a question. Looks like this does what I expect.

@eseliger
Copy link
Member Author

eseliger commented Mar 2, 2020

Thanks for jumping in @dmitshur! I am going to close this PR and will recreate it in the coming hours or days, as the new campaign rollout test will originate from our dogfood instance.

@eseliger eseliger closed this Mar 2, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Mar 2, 2020

The go directive affects only the Go packages in the module it's in.

Based on what you said, setting it to a lower value like go 1.13 may be a better fit. The module is tidy as long as the go directive is present, it doesn't have to be exactly 1.14. That was just the default value.

@keegancsmith keegancsmith deleted the go-mod-tidy-sourcegraph branch March 2, 2020 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants