Page MenuHomePhabricator

Add support for downloading branches, commit's and tags from diffusion gui
ClosedPublic

Authored by mmodell on May 19 2016, 9:48 PM.

Details

Maniphest Tasks
T111887: Diffusion replacement for tarfile download from git.wikimedia.org
Reviewers
Paladox
Luke081515
greg
Commits
rPHAB152da2d872c8: This adds support for downloading from our github mirror.
Patch without arc
git checkout -b D234 && curl -L https://phabricator.wikimedia.org/D234?download=true | git apply
Summary

This adds support for downloading from our github mirror.

The two formats supported by downloading are zip and gz (.tar.gz).

All you have to do is add a mirror to diffusion and the download button
will show.

@mmodell help me with most of this so thankyou :).

Change-Id: Ic7ea639dd85e221d045e52710d25465c7a622ef3

Diff Detail

Repository
rPHAB Phabricator
Branch
arcpatch-D234 (branched from wmf/stable)
Lint
Lint ErrorsExcuse: inside class_exists check
SeverityLocationCodeMessage
Errorsrc/applications/diffusion/controller/DiffusionBrowseController.php:1680PHL1Unknown Symbol
Errorsrc/applications/diffusion/controller/DiffusionCommitController.php:967PHL1Unknown Symbol
Errorsrc/applications/diffusion/controller/DiffusionCommitController.php:968XHP5Use of Undeclared Variable
Errorsrc/applications/diffusion/controller/DiffusionRepositoryController.php:261PHL1Unknown Symbol
Errorsrc/applications/project/engine/PhabricatorProjectProfilePanelEngine.php:41PHL1Unknown Symbol
Unit
Unit Tests OK
Build Status
Buildable 576
Build 872: ci-jessieJenkins
Build 868: ci-jessieJenkins
Build 867: arc lint + arc unit

Event Timeline

Paladox updated this revision to Diff 650.May 19 2016, 9:48 PM
Paladox retitled this revision from to Add support for downloading branches, commit's and tags from diffusion gui.
Paladox updated this object.
Paladox edited the test plan for this revision. (Show Details)
Paladox added a subscriber: mmodell.
Paladox updated this revision to Diff 653.May 19 2016, 11:37 PM

Add missing config

Paladox added a project: Diffusion.
bd808 removed a reviewer: bd808.May 23 2016, 8:13 PM
Paladox updated this revision to Diff 669.May 25 2016, 4:48 PM

Make sure the uri is active not disabled and make sure the uri is a mirror no other link.

Luke081515 accepted this revision.May 26 2016, 4:47 PM
Luke081515 edited edge metadata.

Looks ok, and sounds good.

This revision is now accepted and ready to land.May 26 2016, 4:47 PM
demon requested changes to this revision.May 27 2016, 9:12 PM
demon edited edge metadata.

Couple of minor inline nits about wording of the buttons, otherwise lgtm if @mmodell is ok with it.

src/applications/diffusion/controller/DiffusionBrowseController.php
1703

This should probably say something like "Download zip (from Github)" so people aren't surprised about going offsite.

1709

Same.

src/applications/diffusion/controller/DiffusionCommitController.php
978

Same

984

And finally here

This revision now requires changes to proceed.May 27 2016, 9:12 PM
Paladox updated this revision to Diff 674.May 27 2016, 9:25 PM
Paladox edited edge metadata.
Paladox marked 4 inline comments as done.

Address @demon feedback by updating download buttons to say Download (zip|gz) (from Github).

(zip|gz) means zip or gz there two buttons one says zip and the other gz.

Luke081515 accepted this revision.May 27 2016, 9:27 PM
Luke081515 edited edge metadata.
hashar added a subscriber: hashar.

I cant say anything about the code but I like the idea overall. As I understand it that is going to be maintained as a custom hack, seems to be easy to rebase/maintain forward.

@hashar thanks :). Yes an easy custom solution until phabricator implant download support.

It will also only use mirror uri and only active ones.

Paladox updated this revision to Diff 679.Jun 2 2016, 1:39 AM
Paladox edited edge metadata.

Rebased

mmodell updated this revision to Diff 706.Jun 8 2016, 10:47 PM
mmodell edited edge metadata.

Refactored this into an extension

This makes minimal modifications to phabricator core.
I've moved the majority of the code into a separate class.

See: D258

@mmodell thanks for working on this :).

mmodell accepted this revision.Jun 9 2016, 12:27 AM
mmodell edited edge metadata.
mmodell removed a reviewer: demon.
This revision is now accepted and ready to land.Jun 9 2016, 12:27 AM
mmodell commandeered this revision.Jun 9 2016, 12:28 AM
mmodell edited reviewers, added: Paladox; removed: mmodell.
mmodell closed this revision.Jun 9 2016, 12:28 AM