Skip to content

Save gitea specific settings for vscode #32279

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Save gitea specific settings for vscode #32279

wants to merge 2 commits into from

Conversation

@6543
Copy link
Member

@6543 6543 commented Oct 16, 2024 •
edited
Loading

e.g. allow to run tests from IDE

@6543
save preoject specific settings for vscode
d73be9f 
e.g. allow to run tests from IDE
@6543 6543 added the type/docs This PR mainly updates/creates documentation label Oct 16, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 16, 2024
@yp05327
Copy link
Contributor

yp05327 commented Oct 17, 2024 •
edited
Loading

I prefer this change, as I always need to add the TAGS manually in command line, but if I have custom settings, it will be overwritten by this change?
VSCode will save workspace level settings in settings.json, if I have some custom changes, e.g. the font size, how to cover this case, as it is allowed before.

wxiaoguang
wxiaoguang reviewed Oct 17, 2024
View reviewed changes
.gitignore
Comment on lines +17 to +20
.vscode/*
!.vscode/settings.json
!.vscode/launch.json
!.vscode/extensions.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.vscode/*
!.vscode/settings.json
!.vscode/launch.json
!.vscode/extensions.json
.vscode

The gitignore could be kept as-is. It doesn't affected the manually added files.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes but this is the more compliant way!

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think it is more compliant. Instead it just makes thing more complicated.

By using a single .vscode, everything just works well, and the manually added files are always correctly handled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still prefer to only keep the one line .vscode just like the old

@a1012112796
Copy link
Member

this file has been add in here now.

gitea/contrib/ide/vscode/settings.json

Lines 1 to 4 in 0196b35

{
"go.buildTags": "'sqlite sqlite_unlock_notify'",
"go.testFlags": ["-v"]
}

@yp05327
Copy link
Contributor

yp05327 commented Oct 17, 2024

Ah, we already have it. So this PR is unnecessary?

@6543
Copy link
Member Author

6543 commented Oct 17, 2024

for beginner it's easy to just clone and start ... not sure why we should complicate onboarding ...

@wxiaoguang
Copy link
Contributor

for beginner it's easy to just clone and start ... not sure why we should complicate onboarding ...

It would cause a lot of conflicts for developers who want to modify .vscode/settings.json, unless you could be 100% sure that nobody would edit the settings.json for their personal usage.

@yp05327
Copy link
Contributor

yp05327 commented Oct 17, 2024 •
edited
Loading

I have said here. #32279 (comment)
You can not change the workspace level settings any more. As VSCode will automatically edit this file. I think this should be blocked.

@wxiaoguang
Copy link
Contributor

Inactive for long time.

Again: I do not think this change is right for developers who need to write their own settings.json

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Nov 10, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 29, 2024 •
edited
Loading

@6543 still I do not think it is right (unless the developers never edit this setting file locally). Correct me if I am wrong since I am not a heavy VSCode user.

Update: approved.

wxiaoguang
wxiaoguang approved these changes Dec 29, 2024
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

https://code.visualstudio.com/docs/getstarted/settings

VS Code stores workspace settings at the root of the project in a .vscode folder. This makes it easy to share settings with others in a version-controlled (for example, Git) project.

So maybe it is fine for this case.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 29, 2024
hiifong
hiifong reviewed Dec 29, 2024
View reviewed changes
.vscode/settings.json
Copy link
Member

Choose a reason for hiding this comment

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

This configuration already exists in the contrib/ide/vscode/settings.json

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, only need to keep one

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the pr is not necessary

@hiifong
Copy link
Member

hiifong commented Dec 29, 2024

for beginner it's easy to just clone and start ... not sure why we should complicate onboarding ...

Maybe it would be better if we move the contrib/ide/vscode folder to .vscode in the project root directory.

@wxiaoguang
Copy link
Contributor

@6543 ping ~~~~~~

@wxiaoguang
Copy link
Contributor

@6543 ping ~~~~~~

@6543 ping ~~~~~~

@wxiaoguang
Copy link
Contributor

@6543 ping ~~~~~~

@6543 ping ~~~~~~

@6543 ping

@wxiaoguang
Copy link
Contributor

@6543 ping ~~~~~~

@6543 ping ~~~~~~

@6543 ping

@6543 ping

@wxiaoguang
Copy link
Contributor

@6543 ping ~~~~~~

@6543 ping ~~~~~~

@6543 ping

@6543 ping

@6543 ping

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

+1 more reviewer

@hiifong hiifong hiifong left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 1 This PR needs approval from one additional maintainer to be merged. type/docs This PR mainly updates/creates documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@6543 @yp05327 @a1012112796 @wxiaoguang @hiifong @techknowlogick @GiteaBot