Skip to content

gitignore edits #16

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

Merged
merged 3 commits into from
May 29, 2025
Merged

gitignore edits #16

merged 3 commits into from
May 29, 2025

Conversation

jmwelch-tamu
Copy link
Contributor

added .vscode/setting.json to gitignore, removed settings.json from repo, leaving .vscode folder

@jmwelch-tamu jmwelch-tamu requested a review from bronius May 28, 2025 15:59
Copy link
Contributor

@bronius bronius left a comment

Choose a reason for hiding this comment

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

Was going to suggest ignoring the whole folder but then thought of the files that are useful to share in a project. Per this s.o, they recommend:

.vscode
!.vscode/launch.json

Another article suggested not to pepper every single project we encounter with .vscode but to put it in a local, global. I don't agree with this..

We don't currently have any shared launch configurations, but in a full application, we typically might. For now, let's do the whole .vscode folder and not worry about ignoring launch.json. But keep the deleted settings.json. If there's anything else in the folder, it could all get deleted (but I don't think there is).

@jmwelch-tamu
Copy link
Contributor Author

I am not going to leave the setting.json as part of the repo. Why leave the file checked into the repo, if it's now set to ignore? This way when others download the repo, they don't get any custom settings designed for a user.

I thought about ignoring the whole folder, but considered future options that should/could be shared, since this is a template project.

New commit ignores entire .vscode folder.

@jmwelch-tamu jmwelch-tamu requested a review from bronius May 28, 2025 18:43
Copy link
Contributor

@bronius bronius left a comment

Choose a reason for hiding this comment

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

If you agree with this change, you can also click the "merge when ready" after submitting the last change. I'll click Approve, and it will automatically merge for you.

Co-authored-by: Bronius Motekaitis <[email protected]>
@jmwelch-tamu jmwelch-tamu merged commit 02e098e into main May 29, 2025
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.

2 participants