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.

Sign up for GitHub

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

chore: add vscode workspace settings #7173

Merged
merged 11 commits into from
Oct 22, 2024
Merged

chore: add vscode workspace settings #7173

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c0f2749
Add vscode workspace settings
nazarhussain Oct 17, 2024
5a8c586
Update the settings file
nazarhussain Oct 17, 2024
82d3665
Add settings for open-in-github
nazarhussain Oct 17, 2024
27a1344
Update the settings
nazarhussain Oct 17, 2024
f92bd13
Add comment in the settings
nazarhussain Oct 17, 2024
32b8e4d
Update the settings
nazarhussain Oct 18, 2024
fd02f98
Use tabaqa extension to override settings
nazarhussain Oct 18, 2024
f6b4561
Revert "Use tabaqa extension to override settings"
nazarhussain Oct 22, 2024
6d2f0be
Update the settings file
nazarhussain Oct 22, 2024
da801a4
Update settings
nazarhussain Oct 22, 2024
207e28c
Update .vscode/settings.json
nazarhussain Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,4 @@ packages/cli/.git-data.json
.last_build_unixsec
dictionary.dic

temp/
temp/
3 changes: 2 additions & 1 deletion .vscode/extensions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"recommendations": [
"biomejs.biome",
"esbenp.prettier-vscode"
"esbenp.prettier-vscode",
"KalimahApps.tabaqa"
Copy link
Member

Choose a reason for hiding this comment

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

this does not really seem to be used, we should really be careful installing extensions like this

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's a pretty new extension, out of necessity. I checked it's code before using it.

Copy link
Member

@matthewkeil matthewkeil Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

@matthewkeil matthewkeil Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

]
}
13 changes: 12 additions & 1 deletion lodestar.code-workspace → .vscode/tabaqa.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
{
"root": true,
"settings": {
"window.title": "${activeEditorShort}${separator}${rootName}${separator}${profileName}${separator}[${activeRepositoryBranchName}]",
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll.biome": "explicit"
},
"[javascript]": {
"editor.defaultFormatter": "biomejs.biome"
},
"[typescript]": {
"editor.defaultFormatter": "biomejs.biome"
},
"[json]": {
"editor.defaultFormatter": "biomejs.biome"
},
"[jsonc]": {
"editor.defaultFormatter": "biomejs.biome"
}
}
}
}
3 changes: 3 additions & 0 deletions biome.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
"ignore": ["**/lib", "**/.nyc_output", "./packages/*/spec-tests", "**/node_modules", "./packages/*/node_modules/**"]
},
"organizeImports": {
// TODO: We will enable this settings as soon mono-repo support is provided in biome.
// Currently it didn't recognize local packages in repo and sort those higher than npm packages
// https://github.com/biomejs/biome/issues/2228
"enabled": false
},
"linter": {
Expand Down
Loading