chore: add vscode workspace settings #7173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthewkeil
previously approved these changes
Oct 17, 2024
nflaig
reviewed
Oct 17, 2024
Opened PR, to engage others in this discussion to get their opinion. |
nflaig
reviewed
Oct 17, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #7173 +/- ##
=========================================
Coverage 49.21% 49.21%
=========================================
Files 598 598
Lines 39726 39794 +68
Branches 2092 2097 +5
=========================================
+ Hits 19550 19585 +35
- Misses 20136 20169 +33
Partials 40 40 |
Performance Report✔️ no performance regression detected Full benchmark results
|
|
The right solution to this problem is already in discussion microsoft/vscode#40233 |
|
@nflaig @matthewkeil I used an extension called Tabaqa, which have it's own file This way we can have team specific settings in the |
but doesn't this create a local diff if we track settings.json file? I don't see much of a downside putting a few extra settings in there that may be unused if you don't install the extension for it |
|
We are not tracking This way you don't need to remember if you have local changes in |
Ah got it, it's the other way around, not opposed to it other than we should consider if it's worth the extra extension |
We can and should use that extra extension until VSCode had this feature microsoft/vscode#40233 |
nflaig
reviewed
Oct 18, 2024
.vscode/extensions.json
Outdated
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "recommendations": [ | |||
| "biomejs.biome", | |||
| "esbenp.prettier-vscode" | |||
| "esbenp.prettier-vscode", | |||
| "KalimahApps.tabaqa" | |||
There was a problem hiding this comment.
this does not really seem to be used, we should really be careful installing extensions like this
There was a problem hiding this comment.
Yes that's a pretty new extension, out of necessity. I checked it's code before using it.
There was a problem hiding this comment.
Im not a fan of this extension approach... Everyone will need to have that and similar to what nico mentioned they have the same access as vscode which on my machine is a lot.
I think we should default to a few basic setting.json fields that are necessary.
I honestly even prefer that we do not commit a settings.json at all and instead commit recommended.settings.json and recommended.extensions.json. Anyone that has repo issues and knows what they are doing will go into that folder and see them and will know what to do with them.
And that way we can all keep our own settings.json files intact and so can other users
There was a problem hiding this comment.
I agree with the first part of what @matthewkeil mentioned but I don't like the recommended. files, having essential extensions noted + popup seems fine if we keep it to only mandatory extensions.
As per microsoft/vscode#40233, it seems like settings.json even if the feature is implemented should be tracked via git. We can adopt that approach already and if we don't want any extra stuff in there I can find a way to gitignore my local changes to it, although imo it does not hurt to have 1-2 settingsi n there which would require an extension, those are ignored anyways if you don't have the extension, there is no harm.
There was a problem hiding this comment.
I suppose I can live with that if its only biome and prettier. Tabaqa and the settings file should not be included in this PR
This is the best solution... we should just wait till its part of vscode and only suggest what should be used but not commit/enforce it or extensions that are unwanted |
This reverts commit fd02f98.
nflaig
reviewed
Oct 22, 2024
nflaig
reviewed
Oct 22, 2024
Co-authored-by: Nico Flaig <[email protected]>
nflaig
approved these changes
Oct 22, 2024
| "window.title": "${activeEditorShort}${separator}${rootName}${separator}${profileName}${separator}[${activeRepositoryBranchName}]", | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode", | ||
| // For `sysoev.vscode-open-in-github` extension | ||
| "openInGitHub.defaultBranch": "unstable", |
There was a problem hiding this comment.
As noted in previous comment, not a fan of putting this in the middle of editor. settings but I guess it's fine
There was a problem hiding this comment.
LOL... its funny that this PR had more discussion than a lot
matthewkeil
approved these changes
Oct 22, 2024
| "window.title": "${activeEditorShort}${separator}${rootName}${separator}${profileName}${separator}[${activeRepositoryBranchName}]", | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode", | ||
| // For `sysoev.vscode-open-in-github` extension | ||
| "openInGitHub.defaultBranch": "unstable", |
There was a problem hiding this comment.
LOL... its funny that this PR had more discussion than a lot
|
🎉 This PR is included in v1.23.0 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Share goodies with all the team.
Description
.vscode/settings.jsonfile is the only way to share single root workspace settings among team.Steps to test or reproduce
Additional Notes
From the VSCode