-
Notifications
You must be signed in to change notification settings - Fork 914
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
Remove go dep entirely #3538
Remove go dep entirely #3538
Conversation
ARG GOLANG_VERSION=1.16.2 | ||
ARG GOLANG_CHECKSUM=542e936b19542e62679766194364f45141fde55169db2d8d01046555ca9eb4b8 | ||
ENV PATH=/opt/go/bin:$PATH \ | ||
GOPATH=/opt/go/gopath |
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.
This one I wasn't entirely sure of... there were other references to gopath
folder, but from a quick skim they looked unrelated to this particular folder... it's tricky because gopath
is also a convenient name to use for a generic folder of where go
stuff is.
At a higher level the GOPATH
concept is going away completely in the upcoming go 1.17
: https://blog.golang.org/go116-module-changes so it'd be best to make it so dependabot works w/o needing GOPATH
(as traditionally used by go
) to be set.
I think we should just see what CI says, and if CI greenlights this, then we are probably good to go (assuming CI uses this Dockerfile).
I think I got all the references, but not sure... grep'ing for |
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 can't delete this just yet, we still have users that rely on this in dependabot-preview, we'll need to handle this a little more gracefully than just deleting the code.
No problem, if you need to run a more formal migration process I understand. |
Let's revisit this PR after August 3. I'm looking forward to merging this. |
Rebased to fix merge conflicts now that August 3rd is upon us. @xlgmokha @jurre @thepwagner this is ready to go. 😜 |
Also, any reviewers please grep the remaining code for The risk is that tests could pass but we leave dead code in that isn't caught. |
This has been deprecated for a while, both here in dependabot and also in the `go` ecosystem. The ecosystem has moved to `go mod` (which we already support). So remove the `go dep` code entirely. No need to keep supporting it. Anyone who wants this old of code is likely not keeping up with their dependencies anyway, so probably will just keep using an old version of `dependabot`. See also #3529.
Thanks @jeffwidman, I grepped around for a bit and I don't think there's anything left behind. I also checked some downstream services that depend on dependabot-core and I don't think we have anything that depends on |
Awesome! Thanks @jurre |
The `go` ecosystem has standardized on `go modules`. We removed support for `go dep` entirely from dependabot over in dependabot/dependabot-core#3538, but forgot to update here. So this removes all the `go dep` mentions.
The `go` ecosystem has standardized on `go modules`. We removed support for `go dep` entirely from dependabot over in dependabot/dependabot-core#3538, but forgot to update here. So this removes all the `go dep` mentions.
This has been deprecated for a while, both here in dependabot and also
in the
go
ecosystem. The ecosystem has moved togo mod
(which wealready support).
So remove the
go dep
code entirely. No need to keep supporting it.Anyone who wants this old of code is likely not keeping up with their
dependencies anyway, so probably will just keep using an old version of
dependabot
.See also #3529.