chore: replace govendor with go mod and update docs #647

yeya24 · 2019-07-02T11:30:33Z

Ⅰ. Describe what this PR did

Replace govendor with go mod since we have already updated to go 1.12.6.

Ⅱ. Does this pull request fix one issue?

fix second part of #634

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

pouchrobot · 2019-07-02T11:30:38Z

Thanks for your contribution. 🍻 @yeya24
While we thought PR TITLE could be more specific, longer than 20 chars.
Please edit this PR title instead of opening a new one.
More details, please refer to https://github.com/dragonflyoss/Dragonfly/blob/master/CONTRIBUTING.md

allencloud · 2019-07-02T13:03:15Z

It is a big change to the project.
I am wondering if there is any possibility to check the go mod file's correctness via Makefile target?
If we can do that, we could validate the dependency's correctness.
Just my little opinion. Wish your feedback. @yeya24

yeya24 · 2019-07-02T13:51:38Z

It is a big change to the project.
I am wondering if there is any possibility to check the go mod file's correctness via Makefile target?
If we can do that, we could validate the dependency's correctness.
Just my little opinion. Wish your feedback. @yeya24

@allencloud
Yes this validation is important. And I see this Makefile in Prometheus is like doing what we want. Is this correct?
I change title to WIP and I will try to work on this.

inoc603 · 2019-07-03T06:52:21Z

Since we're hopping on the go mod train, maybe we can consider not checking the vendor/ directory into git repo. go.sum and go.mod can pin dependency versions. Removing vendor/ will prevent huge PRs when changing dependencies. Also we don't have to validate whether vendor/ and go.sum matches.

Of course not checking in vendor/ means we would spend more time downloading dependencies during CI. This can be solved by either:

Make an image with dependencies, which is automatically updated when go.sum and go.mod changes, and do compiling in it.
Or use a proxy that supports the module proxy protocol, some options can be found here

yeya24 · 2019-07-03T07:17:49Z

@allencloud @lowzj WDYT?

starnop · 2019-07-09T08:54:30Z

@lowzj @allencloud @inoc603 Now that we have updated the go version to 1.12.6, so let's make this PR as a break change for release 0.4.1. WDYT?

starnop · 2019-07-09T10:23:12Z

Since we're hopping on the go mod train, maybe we can consider not checking the vendor/ directory into git repo. go.sum and go.mod can pin dependency versions. Removing vendor/ will prevent huge PRs when changing dependencies. Also we don't have to validate whether vendor/ and go.sum matches.

Of course not checking in vendor/ means we would spend more time downloading dependencies during CI. This can be solved by either:

Make an image with dependencies, which is automatically updated when go.sum and go.mod changes, and do compiling in it.

Or use a proxy that supports the module proxy protocol, some options can be found here

I agree with that. Even though go mod now supports the vendor, however, it will be abandoned in the future. And as @inoc603 said, we can use docker image to provide repeatable environments.

yeya24 · 2019-07-09T15:43:41Z

Currently I met some problems in fixing ci.

gometalinter cannot support go mod without vendor. See issue How to use gometaliner for projects with go modules and no vendor? alecthomas/gometalinter#562. Currently I have to run go mod vendor first then run gometalinter. Maybe we can update to golangci-lint in the future since gometalinter has deprecated.
It will always fail in integration-test. I am still finding the cause.

starnop · 2019-07-10T02:07:19Z

Makefile

@@ -94,13 +100,33 @@ unit-test: build-dirs
 	./hack/unit-test.sh
 .PHONY: unit-test

+integration-test:
+	@go build ./test


s/ @go build ./test/ @go test ./test ? Maybe it's the cause of the failure of the integration test.

Thanks for this help!

starnop · 2019-07-10T02:09:38Z

Makefile

+go-mod-vendor:
+	@echo "Begin to vendor go mod dependency"
+	@go mod vendor
+.PHONY: go-mod-tidy


go-mod-vendor？

starnop · 2019-07-10T02:11:07Z

hack/env.sh

@@ -24,3 +24,6 @@ GOOS=$(go env GOOS)
 GOARCH=$(go env GOARCH)
 export GOOS
 export GOARCH
+
+export GO111MODULE=on
+export GOPROXY=https://goproxy.io


Add a newline at end of file.

codecov-io · 2019-07-10T02:14:09Z

Codecov Report

Merging #647 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #647      +/-   ##
==========================================
+ Coverage   48.23%   48.25%   +0.01%     
==========================================
  Files          99       99              
  Lines        5896     5896              
==========================================
+ Hits         2844     2845       +1     
  Misses       2827     2827              
+ Partials      225      224       -1

Impacted Files	Coverage Δ
supernode/daemon/mgr/scheduler/manager.go	`24.81% <0%> (+0.75%)`	⬆️

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 d33074e...8ae6c74. Read the comment docs.

Signed-off-by: yeya24 <[email protected]>

yeya24 · 2019-07-10T02:21:02Z

@starnop @lowzj Can you review this again? Pass the ci now.

starnop · 2019-07-10T02:35:40Z

LGTM. And @lowzj PTAL again.

allencloud · 2019-07-11T09:08:26Z

Great work. We are moving to go mod. 👍 @yeya24 @starnop

chore: replace govendor with go mod and update docs

* chore: update version Signed-off-by: Gaius <[email protected]> * chore: update git version Signed-off-by: Gaius <[email protected]>

pouchrobot added the size/XXL label Jul 2, 2019

yeya24 changed the title ~~chore: add go mod~~ chore: replace govendor with go mod and update docs Jul 2, 2019

yeya24 changed the title ~~chore: replace govendor with go mod and update docs~~ [WIP] chore: replace govendor with go mod and update docs Jul 2, 2019

yeya24 force-pushed the feature/add-go-mod branch from 82b9a8f to 1f21107 Compare July 9, 2019 09:14

starnop reviewed Jul 10, 2019

View reviewed changes

yeya24 force-pushed the feature/add-go-mod branch from 1a0fbc6 to dac840c Compare July 10, 2019 02:11

starnop reviewed Jul 10, 2019

View reviewed changes

move to go mod

8ae6c74

Signed-off-by: yeya24 <[email protected]>

yeya24 force-pushed the feature/add-go-mod branch from dac840c to 8ae6c74 Compare July 10, 2019 02:17

starnop changed the title ~~[WIP] chore: replace govendor with go mod and update docs~~ chore: replace govendor with go mod and update docs Jul 10, 2019

starnop merged commit 13283a1 into dragonflyoss:master Jul 11, 2019

yeya24 deleted the feature/add-go-mod branch July 11, 2019 09:08

pouchrobot mentioned this pull request Jul 12, 2019

WeeklyReport of Dragonfly from 2019-07-05 to 2019-07-12 #672

Closed

starnop added a commit to starnop/Dragonfly that referenced this pull request Nov 27, 2019

Merge pull request dragonflyoss#647 from yeya24/feature/add-go-mod

98c2c3f

chore: replace govendor with go mod and update docs

inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019

Merge pull request dragonflyoss#647 from yeya24/feature/add-go-mod

74e75f9

chore: replace govendor with go mod and update docs

sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022

chore: update version (dragonflyoss#647)

576ff48

* chore: update version Signed-off-by: Gaius <[email protected]> * chore: update git version Signed-off-by: Gaius <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: replace govendor with go mod and update docs #647

chore: replace govendor with go mod and update docs #647

yeya24 commented Jul 2, 2019

pouchrobot commented Jul 2, 2019

allencloud commented Jul 2, 2019

yeya24 commented Jul 2, 2019

inoc603 commented Jul 3, 2019

yeya24 commented Jul 3, 2019

starnop commented Jul 9, 2019

starnop commented Jul 9, 2019

yeya24 commented Jul 9, 2019 •

edited

Loading

starnop Jul 10, 2019

yeya24 Jul 10, 2019

starnop Jul 10, 2019

yeya24 Jul 10, 2019

starnop Jul 10, 2019

yeya24 Jul 10, 2019

codecov-io commented Jul 10, 2019 •

edited

Loading

yeya24 commented Jul 10, 2019

starnop commented Jul 10, 2019

allencloud commented Jul 11, 2019

chore: replace govendor with go mod and update docs #647

chore: replace govendor with go mod and update docs #647

Conversation

yeya24 commented Jul 2, 2019

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

pouchrobot commented Jul 2, 2019

allencloud commented Jul 2, 2019

yeya24 commented Jul 2, 2019

inoc603 commented Jul 3, 2019

yeya24 commented Jul 3, 2019

starnop commented Jul 9, 2019

starnop commented Jul 9, 2019

yeya24 commented Jul 9, 2019 • edited Loading

starnop Jul 10, 2019

Choose a reason for hiding this comment

yeya24 Jul 10, 2019

Choose a reason for hiding this comment

starnop Jul 10, 2019

Choose a reason for hiding this comment

yeya24 Jul 10, 2019

Choose a reason for hiding this comment

starnop Jul 10, 2019

Choose a reason for hiding this comment

yeya24 Jul 10, 2019

Choose a reason for hiding this comment

codecov-io commented Jul 10, 2019 • edited Loading

Codecov Report

yeya24 commented Jul 10, 2019

starnop commented Jul 10, 2019

allencloud commented Jul 11, 2019

yeya24 commented Jul 9, 2019 •

edited

Loading

codecov-io commented Jul 10, 2019 •

edited

Loading