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.

Sign up for GitHub

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

Jump to bottom

Enable GO111MODULE=on globally in Makefile #10939

Merged
merged 3 commits into from
Apr 4, 2020
Merged

Enable GO111MODULE=on globally in Makefile #10939

merged 3 commits into from
Apr 4, 2020

Conversation

mrsdizzie
Copy link
Member

Now that our minimum required go version is 1.12 we can remove code that disables GO111MODULE globally and turn it on, so each command doesn't need it set specifically.

Also fixes a small bug with make pr that didn't work because GO111MODULE=on wasn't set

@mrsdizzie
Enable GO111MODULE=on globally in Makefile
67ca739 
Now that our minimum required go version is 1.12 we can remove code that disables GO111MODULE globally and turn it on, so each command doesn't need it set specifically.

Also fixes a small bug with make pr that didn't work because GO111MODULE=on wasn't set
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Apr 3, 2020
@techknowlogick techknowlogick added this to the 1.12.0 milestone Apr 3, 2020
@techknowlogick techknowlogick added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 3, 2020
@lafriks
Copy link
Member

lafriks commented Apr 3, 2020 •
edited
Loading 

go run -mod=vendor github.com/go-swagger/go-swagger/cmd/swagger generate

is now failing because of this

@mrsdizzie
Copy link
Member Author

This is actually failing because with GO111MODULE=on it tries to vendor the tools we download with go get (revive in this case). Will get fixed in another PR

lunny
lunny approved these changes Apr 3, 2020
View reviewed changes
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

Approved if CI PASS

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 3, 2020
jolheiser
jolheiser approved these changes Apr 3, 2020
View reviewed changes
Copy link
Member

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

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

Approved, waiting on #10942

@jolheiser jolheiser mentioned this pull request Apr 4, 2020
Merged
@guillep2k
Copy link
Member 

+ make coverage
go: cloud.google.com/[email protected]: module lookup disabled by GOPROXY=off
make: *** [Makefile:296: coverage] Error 1

🙁

@mrsdizzie
Copy link
Member Author

@guillep2k waiting on #10947 for that

@lunny
Copy link
Member

lunny commented Apr 4, 2020

make L-G-T-M work

@lunny lunny merged commit 2a06d3a into go-gitea:master Apr 4, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@techknowlogick techknowlogick techknowlogick approved these changes

@jolheiser jolheiser jolheiser approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@mrsdizzie @lafriks @guillep2k @lunny @techknowlogick @jolheiser @GiteaBot