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

Jump to bottom

Gerrit: references to GitHub PRs should be linkified for all valid GitHub repo names, even ones with a dash character #39181

Closed
stamblerre opened this issue May 20, 2020 · 6 comments
Closed

Gerrit: references to GitHub PRs should be linkified for all valid GitHub repo names, even ones with a dash character #39181

stamblerre opened this issue May 20, 2020 · 6 comments
Assignees
@andybons
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@stamblerre
Copy link
Contributor

stamblerre commented May 20, 2020 •
edited

Currently, mirrored PRs only have short references to the original PR, not actual links (for example, https://golang.org/cl/234717). This makes it difficult to navigate between the two. The golang/go#1234 links should be made clickable like links to issues.
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label May 20, 2020
@gopherbot gopherbot added this to the Unreleased milestone May 20, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 21, 2020
@cagedmantis
Copy link
Contributor

/cc @andybons @dmitshur @toothrot

@dmitshur
Copy link
Contributor

This already works for most repositories. For example, CL 234658 was imported from #39176, and Gerrit correctly linkifies the PR mention:

image

image

I suspect there's regexp pattern used to match these, and the pattern currently rejects repository names like "vscode-go" that contain dashes. We should update the pattern in Gerrit to match all valid GitHub repository names.

@dmitshur dmitshur changed the title x/build/cmd/gerritbot: CLs mirrored from PRs should have links back to the PR Gerrit: references to GitHub PRs should work for all valid GitHub repo names, even ones with a dash character May 21, 2020
@stamblerre
Copy link
Contributor Author

Ah, thanks you're just ahead of me. I had just noticed that issues were also not getting links, but gopherbot's cl2issues task worked fine. Where is the regex defined?

@dmitshur dmitshur changed the title Gerrit: references to GitHub PRs should work for all valid GitHub repo names, even ones with a dash character Gerrit: references to GitHub PRs should be linkified for all valid GitHub repo names, even ones with a dash character May 21, 2020
@dmitshur
Copy link
Contributor

I don't know exactly without first looking more, but it should be somewhere in the repository's Gerrit configuration. E.g., see https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#commentlink.name.match.

@andybons
Copy link
Member

It’s in an internal gerrit.config file specific to the Go host.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 18, 2022
@dmitshur
Copy link
Contributor

The relevant commentlink match regexp in Go host's internal gerrit config by now includes "-" as one of the possible character names in org/repo names, so a mention like golang/vscode-go#82 (from CL 234717 in the original issue report) is correctly linked to https://github.com/golang/vscode-go/pull/82:

image

Closing as fixed. (It was actually fixed on May 26, 2020 via cl/313222876, we just forgot to close this issue.)

@golang golang locked and limited conversation to collaborators Jul 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@andybons andybons

Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@cagedmantis @andybons @dmitshur @stamblerre @gopherbot