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

Add .gitignore and fix a few typos #35

Closed
wants to merge 1 commit into from

Conversation

ehsundar
Copy link

No description provided.

Copy link
Owner

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

Thanks for your interest on my package, and thanks for catching the typo and opening a PR.

While I'm ok with the change about the typo, I would decline the addition of .gitignore file.

While the change you introduce seems to be legitimate. You have to consider what could be a good .gitignore for you, won't be mine, or someone else one. There are many .gitignore generator, indeed. But adding their content to a project is according to me is not the right way to do.

Such .gitignore is good and valid, I do have one locally too. But I use the git config setting core.excludesFiles, so this file is out of the repository

You can find some information here

https://dev.to/jt_ziolo/how-to-create-a-global-gitignore-file-as-an-extra-precaution-against-committing-secrets-and-unnecessary-files-3hbn

I personally use a file named ~/.config/git/gitignore_global

@@ -1,6 +1,6 @@
package safecast

// This files is highly inspired from https://pkg.go.dev/golang.org/x/exp/constraints
// These files are highly inspired from https://pkg.go.dev/golang.org/x/exp/constraints
Copy link
Collaborator

Choose a reason for hiding this comment

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

afaik it's only here that the types are set, thus the original comment is almost correct (modulo files->file):

Suggested change
// These files are highly inspired from https://pkg.go.dev/golang.org/x/exp/constraints
// This file is highly inspired from https://pkg.go.dev/golang.org/x/exp/constraints

@ehsundar
Copy link
Author

Thanks for your comments. I close the PR then.

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