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

Remove .vscode/settings.json #97

Merged
merged 3 commits into from
Sep 24, 2024
Merged

Remove .vscode/settings.json #97

merged 3 commits into from
Sep 24, 2024

Conversation

cc-a
Copy link
Contributor

@cc-a cc-a commented Sep 20, 2024

Removes vscode settings file and adds to .gitignore.

Having the settings commited requires developers to have a dirty worktree in order to customise project specific settings.

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (235f980) to head (96e7210).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #97   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines           43        43           
=========================================
  Hits            43        43           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

In general, the point of having this committed is for the developers contributing to a specific project to have the same settings to improve consistency and NOT customise them at workspace level. If they want customisation, they can do so at User level, right?

@cc-a
Copy link
Contributor Author

cc-a commented Sep 20, 2024

Consistency is enforced via CI. I'd argue the developer should be free to have whatever development experience they want in their editor without being forced to configure things at the user level which doesn't have project isolation.

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

OK, then.

Copy link
Contributor

@jamesturner246 jamesturner246 left a comment

Choose a reason for hiding this comment

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

No strong opinion here 👍

Copy link
Contributor

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

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

I'm not convinced. I get that it's a bit weird, but it's pretty common practice to commit the settings.json file to git (e.g. here's the one in VS Code's own repo). It's handy to be able to set the default formatter for Python files etc. Yes, we enforce it properly with pre-commit/CI, but this is really just for convenience.

That said, there are clearly other people out there who want to be able to override the workspace settings.json without having a dirty git tree, which seems a sensible proposal: microsoft/vscode#40233

There aren't that many settings in the file, so I don't think it matters too much if we drop it (but I will continue to commit settings.json for my own projects 😉). Up to you!

@cc-a cc-a marked this pull request as ready for review September 24, 2024 09:40
@cc-a cc-a enabled auto-merge September 24, 2024 09:40
@cc-a cc-a merged commit 82de1d9 into main Sep 24, 2024
5 checks passed
@cc-a cc-a deleted the remove-vscode-settings branch September 24, 2024 09:41
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.

5 participants