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.

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

Remove branch URL before IssueRefURL #15968

Merged
merged 1 commit into from
May 25, 2021

Conversation

fnetX
Copy link
Contributor

@fnetX fnetX commented May 24, 2021

My attempt to fix #15967 (duplicate Owner/Branch in URL when linking to a branch from an issue).
It appears to me as if IssueRefURLs already contain the full valid URL by now and thus prepending the Owner and Branch leads to duplication.

Copy link
Member

@noerw noerw left a comment

Choose a reason for hiding this comment

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

Thanks! One nit, see code comment.
Ffor reference; the repoLink is prepended to the URLs here already

issueRefURLs[issue.ID] = git.RefURL(repoLink, util.PathEscapeSegments(issue.Ref))

templates/shared/issuelist.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 May 24, 2021
@noerw noerw added the type/bug label May 24, 2021
@noerw noerw added this to the 1.15.0 milestone May 24, 2021
@fnetX
Copy link
Contributor Author

fnetX commented May 24, 2021

Ffor reference; the repoLink is prepended to the URLs here already

Looks like it's been broken for a long time, then. Seems like few users rely on it 🤔
I can also create a backport PR.

@noerw
Copy link
Member

noerw commented May 24, 2021

Sorry, please revert my suggestion. The non RepoLink case is still needed for the issue listing in the account / org dashboard

@fnetX
Copy link
Contributor Author

fnetX commented May 24, 2021

Makes sense to me now, thank you. I should have given the cases a second thought, I just assumed that the data was now contained globally, because both cases had it prepended before.

Revert change for account / org dashboard where IssueRefURLs do not
contain the full repo URL (case RepoLink is not true)

Co-authored-by: Norwin <noerw@users.noreply.github.com>

Remove trailing whitespace from PR review
@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 May 25, 2021
@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 May 25, 2021
@techknowlogick techknowlogick merged commit d5f2010 into go-gitea:main May 25, 2021
@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label May 25, 2021
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 10, 2021
Revert change for account / org dashboard where IssueRefURLs do not
contain the full repo URL (case RepoLink is not true)

Co-authored-by: Norwin <noerw@users.noreply.github.com>

Remove trailing whitespace from PR review
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Branch reference from issue displays 404 Not Found
5 participants