Skip to content

Revert "Workspace/InitPackage: Add .vscode to .gitignore" #8168

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 1 commit into from
Dec 16, 2024

Conversation

plemarquand
Copy link
Contributor

Reverts #8164

@@ -439,7 +439,6 @@ public final class InitPackage {
.swiftpm/configuration/registries.json
.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata
.netrc
.vscode
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT of just changing this to .vscode/launch.json?

Copy link
Member

Choose a reason for hiding this comment

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

Teams may want to share custom launch configurations. Mahdi makes some good points on the original PR as well. At the very least, we need to think about this a bit more.

Copy link
Contributor

Choose a reason for hiding this comment

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

My objection here is that auto-generated files should not be committed in the first place. I've bit gitignoring this file in my every single Swift package because VS Code kept recreating it with no way to prevent that, and I don't see a use case for keeping it really. At the very least, VS Code should keep auto-generated and manually maintained data in separate files.

Copy link
Contributor Author

@plemarquand plemarquand Dec 9, 2024

Choose a reason for hiding this comment

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

What about something like:

.vscode/*
!.vscode/settings.json
!.vscode/tasks.json
!.vscode/launch.json
!.vscode/extensions.json
!.vscode/*.code-snippets

Those un-ignored files prefixed with ! are some you may want to share. launch.json, like tasks.json, can be shared and may be potentially useful to others.

Copy link
Member

Choose a reason for hiding this comment

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

Same, every project I use VS Code with, I add this line into gitignore.

I'm curious, to make this more data-driven: could we search for packages that have Package.swift and also either have .vscode in the gitignore, or have at least one checked-in file from the .vscode directory? That way we could see what's more common and which group to optimize for (and the other group can always update their gitignore post-init).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unfamiliar with some of the xcode related entries in the gitignore. Seems like xcuserdata makes sense to ignore. I'm not sure about contents.xcworkspacedata.

Is the underlying issue that the VS Code Swift Extension is creating and modifying launch/tasks.json files in the .vscode folder? If so I think it makes sense to address this in the extension and if possible avoid auto-generating these configurations. That would mean that anything in .vscode would be put there explicitly by the user and then its totally up to them how to set up their .gitconfig around .vscode.

Copy link
Member

Choose a reason for hiding this comment

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

Is the underlying issue that the VS Code Swift Extension is creating and modifying launch/tasks.json files in the .vscode folder? If so I think it makes sense to address this in the extension and if possible avoid auto-generating these configurations. That would mean that anything in .vscode would be put there explicitly by the user and then its totally up to them how to set up their .gitconfig around .vscode.

I'm all for that. I actually find the autogenerated tasks for SwiftPM to be annoying. There's so many of them. That said I do appreciate help creating them. I'd hate to have to remember where my executable ended up.

Copy link
Member

Choose a reason for hiding this comment

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

I'm unfamiliar with some of the xcode related entries in the gitignore. Seems like xcuserdata makes sense to ignore. I'm not sure about contents.xcworkspacedata.

Right, might make sense to declare bankruptcy and remove them all, that way at least we start from a consistent point.

xcuserdata is user data, but the argument for not having .vscode was that some people do want their user data committed, so I'm curious if we can get to a set of rules that help contributors understand what is and isn't in the gitignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@czechboy0 While you can configure whats in .vscode to be user data specific to a user I'd argue that its mainly for project configuration data. Settings, recommended extensions, launch tasks, etc are all project specific and not user specific. They can all be configured with relative paths so that when a new user checks out the project it builds in VS Code with no extra setup. I'd argue the default is to commit it and the less common path is to have non-portable configuration in there that should be .gitignored. If the VS Code Swift extension is generating launch configs that aren't portable or useful then we should have that discussion in that repository or on the forums.

For now I propose we go forward with this revert and continue to discuss.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, go ahead. I've personally not worked on a single repo that checked in .vscode contents, all the ones I've worked with ignore it, but it's also not too difficult to add it to .gitignore on every project I work on, so happy with the revert 👍

@MaxDesiatov MaxDesiatov requested a review from czechboy0 December 9, 2024 16:39
@plemarquand
Copy link
Contributor Author

@swift-ci test

@plemarquand
Copy link
Contributor Author

@swift-ci test windows

Copy link
Member

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

See my comment here, I think we should follow what the majority of packages prefer: #8168

See clarification in the comment

@czechboy0 czechboy0 self-requested a review December 11, 2024 17:00
@plemarquand plemarquand merged commit a19fd17 into main Dec 16, 2024
5 checks passed
@plemarquand plemarquand deleted the revert-8164-hd-add-vscode-to-gitignore branch December 16, 2024 15:43
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.

7 participants