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

pkg:generator: Generate .gitignore at top and for tmp/_output #232

Merged
merged 1 commit into from
May 10, 2018

Conversation

eparis
Copy link
Contributor

@eparis eparis commented May 7, 2018

The top level ignore golang, vim, and emacs files
The tmp/.gitignore ignores tmp/_output

@unterstein
Copy link

Hey,

this would fix #185 🎉 It would be nice to add the following entries as well:

vendor
tmp
.idea
.vscode/

There is a .gitignore in the samples: https://github.com/operator-framework/operator-sdk-samples/blob/master/vault-operator/.gitignore

Thanks


package generator

const tmpGitignoreTmpl = `_output`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we let the .gitignore at the project root to ignore the tmp/_output folder instead?

e.g:

# Folders
tmp/_output/

.netrwhist
# auto-generated tag files
tags

Copy link
Contributor

Choose a reason for hiding this comment

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

probably want to ignore the following as well:

### Build Folders
vendor/
tmp/_output/

@@ -266,6 +296,16 @@ func (g *Generator) renderTmp() error {
return renderCodegenFiles(cDir, g.repoPath, apiDirName(g.apiVersion), version(g.apiVersion), g.projectName)
}

func renderTmpGitignore(tDir string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is necessary. Just let the project root .gitignore handle all the ignoring files.

@eparis
Copy link
Contributor Author

eparis commented May 10, 2018

I addressed the comments, though I did not include vendor. Why wouldn't we suggest people include vendor? I know that the dep lock should allow it to be recreated, but if you want people to be able to build what you wrote...

@eparis
Copy link
Contributor Author

eparis commented May 10, 2018

Oh, I didn't include "tmp" either. It (like vendor) could easily be a followup...

@fanminshi
Copy link
Contributor

fanminshi commented May 10, 2018

Why wouldn't we suggest people include vendor?
You have a point. I guess we should let users themselves to decide if they want to include vendor or not.

Any reason to have to two .gitignore files?
one in root and on in the pkg/tmp?

The top level ignore goland, vim, and emacs files
The tmp/.gitignore ignores tmp/_output
@eparis
Copy link
Contributor Author

eparis commented May 10, 2018

I removed the 2nd .gitignore but forgot to remove the 2nd "template". It's gone now. I think this can merge and we can enhance it in other PRs.

@fanminshi
Copy link
Contributor

lgtm Thanks!

@fanminshi fanminshi merged commit 79d2ee5 into operator-framework:master May 10, 2018
@eparis eparis deleted the gitignore branch May 14, 2018 18:15
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