Skip to content

Add new makefile target cleanall to remove files pulled down by bootstrap.sh#3411

Merged
sougou merged 2 commits intovitessio:masterfrom
sjmudd:makefile_cleanall
Dec 4, 2017
Merged

Add new makefile target cleanall to remove files pulled down by bootstrap.sh#3411
sougou merged 2 commits intovitessio:masterfrom
sjmudd:makefile_cleanall

Conversation

@sjmudd
Copy link
Copy Markdown
Contributor

@sjmudd sjmudd commented Nov 24, 2017

This is to completely clean up your working environment without touching anything that's local in the git repo. Reasons for this are when changing the go binaries or wanting to be sure your repo and build environment is completely clean.

…trap.sh

This is to completely clean up your working environment without touching
anything that's local in the git repo. Reasons for this are when changing
the go binaries or wanting to be sure your repo is clean.
@sjmudd
Copy link
Copy Markdown
Contributor Author

sjmudd commented Nov 24, 2017

Maybe distclean is a better target name. not completely sure. cleanall seems clear to me.

# keep the vendor.json file but nothing else under the vendor directory as it's not actually part of the Vitess repo
rm -rf vendor/cloud.google.com vendor/github.com vendor/golang.org vendor/google.golang.org vendor/gopkg.in
# other stuff in the go hierarchy that is not under vendor/
rm -rf ../../../golang.org ../../../honnef.co
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We shouldn't remove anything outside the vitess tree because the user may have other projects that depend on those downloads. Same for the line below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not removing anything that wasn't created by bootstrap.sh. If someone has added local changes I agree they'll lose them but isn't something like make cleanall intended to do what it says?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you want me to be very explicit and check each and every package that's pulled down by bootstrap.sh then I can do that. It's not clear to me what you're trying to prevent or if you just want to be extremely cautious when running such a command.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was more worried about such external dependencies being used by other projects. But I guess such people can stay away from using cleanall.

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Dec 4, 2017

LGTM

Approved with PullApprove

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants