Page MenuHomePhabricator

Add more redirection url patterns for gerrit urls
ClosedPublic

Authored by mmodell on Dec 1 2015, 5:24 PM.

Details

Maniphest Tasks
T110607: redirect gerrit repo paths to diffusion callsigns
Reviewers
demon
Commits
rPHEXd724c51a4144: Add more redirection url patterns for gerrit urls
Patch without arc
git checkout -b D68 && curl -L https://phabricator.wikimedia.org/D68?download=true | git apply
Summary

This corresponds to the following gerrit configuration:

[gitweb]
    url = https://git.wikimedia.org
    type = custom
    project = /r/project/${project}
    revision = /r/revision/${project};${commit}
    branch = /r/branch/${project};${branch}
    filehistory = /r/browse/${project};${branch};${file}
    linkname = gitblit
    linkDrafts = false
Test Plan

Test on phab-01.wmflabs.org:

Browse gerrit project, redirects to callsign:

Browse a file .gitreview in project mediawiki/core on the REL1_19 branch:

Browse history for mediawiki/core branch REL1_19:

Browse to a specific commit:

Diff Detail

Repository
rPHEX phabricator-extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mmodell updated this revision to Diff 201.Dec 1 2015, 5:24 PM
mmodell retitled this revision from to Add more redirection url patterns for gerrit urls.
mmodell updated this object.
mmodell edited the test plan for this revision. (Show Details)
mmodell added a reviewer: demon.
mmodell edited the test plan for this revision. (Show Details)Dec 1 2015, 5:36 PM
mmodell edited the test plan for this revision. (Show Details)Dec 1 2015, 5:51 PM
demon edited edge metadata.Dec 1 2015, 5:51 PM

Couple of minor inline bits but otherwise ok.

GerritApplication.php
45

sha1s are only [0-9a-f]+ if we want to be strict about it.

50

[nitpick] array_fill_keys() does exactly what you want here.

mmodell edited the test plan for this revision. (Show Details)Dec 1 2015, 5:55 PM
mmodell edited edge metadata.
mmodell updated this revision to Diff 202.Dec 1 2015, 5:56 PM

commits didn't match the right regex. All the patterns should work now.

mmodell updated this revision to Diff 203.Dec 1 2015, 5:59 PM
mmodell marked an inline comment as done.

use array_fill_keys()

mmodell marked an inline comment as done.Dec 1 2015, 5:59 PM
mmodell added inline comments.
GerritApplication.php
45

It doesn't need to be terribly strict. I just copied the pattern that was used in the diffusion application routes, for consistency.

mmodell marked an inline comment as done.Dec 1 2015, 6:02 PM

@demon: I believe I have addressed your concerns. Land it?

demon accepted this revision.Dec 1 2015, 6:02 PM
demon edited edge metadata.
This revision is now accepted and ready to land.Dec 1 2015, 6:02 PM
This revision was automatically updated to reflect the committed changes.
Paladox added a subscriber: Paladox.Dec 6 2015, 1:16 PM
Paladox added inline comments.
GerritApplication.php
43

Does not work the url looks like https://phabricator.wikimedia.org/r/branch/mediawiki/core in gerrit https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/core,branches it doesn't show the correct link for example it should look like https://phabricator.wikimedia.org/r/mediawiki/core/branch/master and then it should redirect to the correct link.

43

Same goes for commit and revision and file history.