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.

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

commands/operator-sdk/cmd/new: initialize git on new #250

Merged
merged 1 commit into from
May 18, 2018

Conversation

eparis
Copy link
Contributor

@eparis eparis commented May 15, 2018

I love that cargo new from the rust project initializes git after it creates
the stock files. I don't have to run that command myself. Make operator-sdk new
just call git init.

@spahl
Copy link
Contributor

spahl commented May 15, 2018

I like it too. What do you think of adding a flag to disable it for the people who would use something else?

@eparis
Copy link
Contributor Author

eparis commented May 15, 2018

added --skip-git-init

@spahl
Copy link
Contributor

spahl commented May 15, 2018

LGTM /cc @hasbro17

one thing: this now has an implicit dependence to git and that should be noted in the README

I love that `cargo new` from the rust project initializes git after it creates
my stock files. I don't have to run that command myself. Make `operator-sdk new`
just call git init.
@eparis
Copy link
Contributor Author

eparis commented May 15, 2018

added git to list of prereqs in the user-guide.

@fanminshi
Copy link
Contributor

lgtm

@hasbro17
Copy link
Contributor

I don't think it's a good idea for operator-sdk new make the initial commit for the user.
Right now the generated .gitignore for a new project does not ignore the vendor directory since we wanted to leave that up to the user. But by making the first commit we're making the choice for the user to always commit the vendor in their repo.
https://github.com/operator-framework/operator-sdk/blob/master/pkg/generator/gitignore_tmpls.go

We touched on this earlier but I don't think we decided to set the convention of enforcing the vendor dir to be committed.
#232 (comment)
https://github.com/golang/dep/blob/master/docs/FAQ.md#should-i-commit-my-vendor-directory

@eparis
Copy link
Contributor Author

eparis commented May 15, 2018

Users can still make that choice, although admittedly harder since they have to blow away the git commit, or use --skip-git-init.

I'd argue that vendor/ is the only way to get something that we know others can download and use. While dep ensure probably will work, even the dep page points out that's not always true. We're setting the default and I think, out of the box, using vendor/ (like all of kube does) should be our default. The real con on the dep website appears to just be: 'its bigger'. But since you need all of that code to build, it's not really bigger. Certainly it is a lot smaller than running dep ensure and dowloading the complete history of every one of those deps!

@hasbro17
Copy link
Contributor

@eparis Fair enough. Committing the vendor does seem to have more benefits and it seems to be the standard for all kube projects as well.

LGTM

@hasbro17 hasbro17 merged commit 8a1704d into operator-framework:master May 18, 2018
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.

None yet

4 participants