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

Make the vscode clone link respect transport protocol (#20557) #21128

Merged
merged 5 commits into from
Sep 20, 2022
Merged

Make the vscode clone link respect transport protocol (#20557) #21128

merged 5 commits into from
Sep 20, 2022

Conversation

ghost
Copy link

@ghost ghost commented Sep 9, 2022

Backports #20557

silverwind
silverwind reviewed Sep 9, 2022
View reviewed changes
templates/repo/home.tmpl 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 Sep 9, 2022
@techknowlogick techknowlogick changed the title Make the vscode clone link respect transport protocol (backport #20557) Make the vscode clone link respect transport protocol (#20557) Sep 9, 2022
noerw
noerw reviewed Sep 10, 2022
View reviewed changes
templates/repo/home.tmpl Outdated Show resolved Hide resolved
@monim67
Copy link

monim67 commented Sep 13, 2022

Is it ready to be merged now?

@noerw
Copy link
Member

noerw commented Sep 13, 2022

Is it ready to be merged now?

Usually we only backport bugfixes, not enhancements.
Also the CI complains and wants the change above.

@noerw noerw added the type/enhancement An improvement of existing functionality label Sep 13, 2022
@monim67
Copy link

monim67 commented Sep 14, 2022

For me it's a bug, because I have HTTPS git disabled but clone in VSCode button always gives me HTTPS link. It never changes to SSH link. I had to override the template to replace HTTPS with SSH link to get it to work. I have to do this everytime a new version is released 😞

@lunny lunny added this to the 1.17.3 milestone Sep 18, 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 Sep 20, 2022
@wxiaoguang
Copy link
Contributor

@monim67-renforce : "This branch is out-of-date with the base branch": please enable Allow Edits from Maintainers, or update with 1.17 branch.

@monim67
Copy link

monim67 commented Sep 20, 2022

@wxiaoguang I cannot give write access as I created the PR from wrong account. None of the account has that allow edit checkbox now. I have updated the branch by merge.

@wxiaoguang
Copy link
Contributor

I see. Just now there is a new commit in 1.17, could you update it again? Then we can get this merged in next time.

@monim67
Copy link

monim67 commented Sep 20, 2022

@wxiaoguang updated again.

@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 Sep 20, 2022
@wxiaoguang wxiaoguang merged commit 54d4e66 into go-gitea:release/v1.17 Sep 20, 2022
@wxiaoguang wxiaoguang mentioned this pull request Sep 21, 2022
Merged
@wxiaoguang
Copy link
Contributor

Another bug: Golang doesn't like the string syntax: `...//...`, it will truncate the // as comment, then result in bad script code.

@wxiaoguang
Copy link
Contributor

#21224 (comment)

wxiaoguang added a commit that referenced this pull request Sep 23, 2022
@wxiaoguang
Make Clone in VSCode link get updated correctly (#21225) (#21226)
be5411d 
Backport #21225, fix for #21128 (also in 1.17.3), close #21224

The indent was incorrect before, so this PR did some formatting work. 

Bypass Golang's template bug for JS string interpolation. And since
there are JS lint rules for templates, so the string interpolation is
also a must.
@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
No reviews
Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.17.3
Development

Successfully merging this pull request may close these issues.
6 participants
@monim67 @noerw @wxiaoguang @silverwind @lunny @GiteaBot