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

Jump to bottom

Use recommended vscode configuration in gitpod environments #21537

Merged
merged 8 commits into from
Oct 24, 2022
Merged

Use recommended vscode configuration in gitpod environments #21537

merged 8 commits into from
Oct 24, 2022

Conversation

yardenshoham
Copy link
Member

To make go tests run in gitpod, the vscode settings.json must be in the correct place in the filesystem

@yardenshoham
Use recommended vscode configuration in gitpod environments
4de340f 
To make go tests run in gitpod, the vscode settings.json must be in the correct place in the filesystem

Signed-off-by: Yarden Shoham <hrsi88@gmail.com>
wxiaoguang
wxiaoguang reviewed Oct 22, 2022
View reviewed changes
.gitpod.yml Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 22, 2022
@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 Oct 22, 2022
@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Oct 22, 2022
@silverwind
Copy link
Member

Why not move it into the correct place? Are the settings in there too opinionated? Note, I'm not a vscode user, so I can't really comment on them.

@yardenshoham
Copy link
Member Author

yardenshoham commented Oct 22, 2022 •
edited

Why not move it into the correct place? Are the settings in there too opinionated? Note, I'm not a vscode user, so I can't really comment on them.

I'm for placing it at the root

@wxiaoguang
Copy link
Contributor

Why not move it into the correct place? Are the settings in there too opinionated? Note, I'm not a vscode user, so I can't really comment on them.

I am against for putting the .vscode in the root. These settings don't satisfy everyone, some people may want to change these settings. If the .vscode is put in the root, then how could they handle these changed files when committing?

@yardenshoham
Copy link
Member Author

These settings are project-specific, any user settings are in another file. Having this at the root helps vscode users and hurts no one. I can't think of any project-specific setting vscode users have that shouldn't be committed.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 22, 2022 •
edited

These settings are project-specific, any user settings are in another file. Having this at the root helps vscode users and hurts no one. I can't think of any project-specific setting vscode users have that shouldn't be committed.

But what if some people modified launch.json with other commands or changed the go.buildTags in settings.json?

And this comment: #2483 (comment)

@yardenshoham
Copy link
Member Author

Then they probably did it for good reason and should commit the modification

@wxiaoguang
Copy link
Contributor

Then they probably did it for good reason and should commit the modification

I do not think so. When I am using VSCode, sometimes I add temporary options for my local debug purpose.

@yardenshoham
Copy link
Member Author

yardenshoham commented Oct 22, 2022 •
edited

If it's temporary, then you would simply not commit it

@wxiaoguang
Copy link
Contributor

Then I have to manually deselect them when committing 😂 again and again.

So, leave an empty .vscode and prepare it for everyone locally is the most clear solution IMO.

@yardenshoham
Copy link
Member Author

Then I have to manually deselect them when committing 😂 again and again.

How is that any different than not committing say, debug log statements?

@wxiaoguang
Copy link
Contributor

  • .vscode in root: ignore them from time to time if there are local changes.
  • vscode in contrib: only copy the directory when first setup if you need (the new developer should have spent enough time on reading the development guideline, a copy won't take too much time), then change it freely as you want. No need to spend more time on it.
  • debug log statements: sometimes they can be avoided. If they can not be avoided, then the time can not be saved.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 22, 2022
silverwind
silverwind approved these changes Oct 22, 2022
View reviewed changes
Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

It's an improvement in any case.

@zeripath zeripath merged commit c04ad76 into go-gitea:main Oct 24, 2022
@yardenshoham yardenshoham deleted the vscode branch October 24, 2022 06:30
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 24, 2022
@zjjhot
Merge remote-tracking branch 'upstream/main'
ebd6917 
* upstream/main:
  adapt README_{Country}.md stype name in localizedExtensions (go-gitea#21486)
  dump: Add option to skip index dirs (go-gitea#21501)
  Use recommended vscode configuration in gitpod environments (go-gitea#21537)
  Expand "Go to File" button again, fix 'Add File' margin (go-gitea#21543)
  Add yardenshoham to maintainers (go-gitea#21566)
  Refactor git command arguments and make all arguments to be safe to be used (go-gitea#21535)
  Update binding to fix bugs (go-gitea#21556)
  Link mentioned user in markdown only if they are visible to viewer (go-gitea#21554)
  Require authentication for OAuth token refresh (go-gitea#21421)
  CSS color enhancements (go-gitea#21534)
  Allow package version sorting (go-gitea#21453)
  Add link to user profile in markdown mention only if user exists (go-gitea#21533)
  Update milestone counters when issue is deleted (go-gitea#21459)
  Prevent Authorization header for presigned LFS urls (go-gitea#21531)
  Remove deleted repos from searchresult (go-gitea#21512)
  Remove unnecessary debug log (go-gitea#21536)
  Added check for disabled Packages (go-gitea#21540)
  Decouple HookTask from Repository (go-gitea#17940)
  Add color previews in markdown (go-gitea#21474)
  Fix generating compare link (go-gitea#21519)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@yardenshoham @silverwind @wxiaoguang @delvh @zeripath @GiteaBot