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

abandon vendor #1337

Merged
merged 7 commits into from Nov 26, 2018

Conversation

Projects
None yet
5 participants
@disksing
Member

disksing commented Nov 22, 2018

Signed-off-by: disksing i@disksing.com

What problem does this PR solve?

Remove vendor directory. Make code base clean.

NOTE: go modules are not well supported by some IDEs. You can manually run go mod vendor to make your editor work. The vendor directory is already added to .gitignore.

What is changed and how it works?

  • remove /vendor
  • update makefile

Check List

Tests

  • Unit test

Side effects

  • Breaking backward compatibility: does not support old version go (earlier than 1.11) anymore
abandon vendor
Signed-off-by: disksing <i@disksing.com>

@disksing disksing requested review from gregwebs, nolouch and rleungx Nov 22, 2018

@disksing disksing added WIP and removed WIP labels Nov 22, 2018

@gregwebs

This comment has been minimized.

Contributor

gregwebs commented Nov 22, 2018

This change is good in principle: the problems are just of adapting workflows. IDE is a good one to point out. The main thing that comes to my mind would be CI or any docker builds. Will the CI go from one quick clone to 50 separate clones now, or will we be able to cached the go modules across builds?

@disksing

This comment has been minimized.

Member

disksing commented Nov 22, 2018

@gregwebs Good point. I'll check if there are any cache options we can use in CI environments.

@disksing disksing added the WIP label Nov 22, 2018

disksing added some commits Nov 22, 2018

ci: support build cache
Signed-off-by: disksing <i@disksing.com>
fix format
Signed-off-by: disksing <i@disksing.com>
@disksing

This comment has been minimized.

Member

disksing commented Nov 22, 2018

/run-all-tests

@disksing

This comment has been minimized.

Member

disksing commented Nov 22, 2018

/run-unit-test

@disksing disksing removed the WIP label Nov 22, 2018

@disksing

This comment has been minimized.

Member

disksing commented Nov 22, 2018

@gregwebs Done for Circle and Travis. Our internal CI doesn't support build cache yet. Sadly.

@gregwebs

This comment has been minimized.

Contributor

gregwebs commented Nov 23, 2018

LGTM then. But internal CI might be a reason not to make this change yet. I think due to tidb doing this and the build slowing down there may be some effort to figure out dependency caching.

@siddontang

LGTM

@disksing disksing merged commit 239dbdd into pingcap:master Nov 26, 2018

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci-pd/build Jenkins job succeeded.
Details
jenkins-ci-pd/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@disksing disksing deleted the disksing:no-vendor branch Nov 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment