Ref T5000. Works fine, but totally unusuable in present form.
Details
Details
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- pushreview
- Lint
Lint Passed Severity Location Code Message Advice src/applications/diffusion/engine/DiffusionCommitHookEngine.php:133 XHP16 TODO Comment Advice src/applications/diffusion/engine/DiffusionCommitHookEngine.php:152 XHP16 TODO Comment Advice src/applications/diffusion/engine/DiffusionCommitHookEngine.php:700 XHP16 TODO Comment - Unit
No Test Coverage - Build Status
Buildable 1173 Build 1173: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
$ git push origin HEAD:review Counting objects: 22, done. Delta compression using up to 8 threads. Compressing objects: 100% (3/3), done. Writing objects: 100% (3/3), 388 bytes | 0 bytes/s, done. Total 3 (delta 2), reused 0 (delta 0) remote: remote: Success! Created a new diff fom your changes: remote: remote: http://local.aphront.com:8080/differential/diff/588/ remote: remote: Since this was a push for review, your changes will not be remote: applied to the remote. This push will now be rejected. remote: To ssh://dweller@localhost/diffusion/POEMS/ ! [remote rejected] HEAD -> review (pre-receive hook declined) error: failed to push some refs to 'ssh://dweller@localhost/diffusion/POEMS/'
Comment Actions
After I apply this patch on the latest codebase, it raises an error.
$ git push origin HEAD:review Counting objects: 3, done. Delta compression using up to 4 threads. Compressing objects: 100% (3/3), done. Writing objects: 100% (3/3), 301 bytes | 0 bytes/s, done. Total 3 (delta 2), reused 0 (delta 0) remote: [2016-01-10 05:55:15] EXCEPTION: (TypeError) Argument 1 passed to DifferentialDiff::newFromRawChanges() must be an instance of PhabricatorUser, array given, called in /Users/Leedy/Developer/github/phacility/phabricator/src/applications/diffusion/engine/DiffusionCommitHookEngine.php on line 747 at [<phabricator>/src/applications/differential/storage/DifferentialDiff.php:140] remote: arcanist(head=master, ref.master=aeb374b33348), phabricator(head=master, ref.master=efba69a44022, custom=2), phutil(head=master, ref.master=adb8a9c074ba) remote: #0 DifferentialDiff::newFromRawChanges(array) called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:747] remote: #1 DiffusionCommitHookEngine::generateGitDiffForReview(PhabricatorRepositoryPushLog) called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:327] remote: #2 DiffusionCommitHookEngine::generateDiffForReview(array) called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:131] remote: #3 DiffusionCommitHookEngine::execute() called at [<phabricator>/scripts/repository/commit_hook.php:133] To http://127.0.0.1:3000/diffusion/FUN/funtudy.git ! [remote rejected] HEAD -> review (pre-receive hook declined) error: failed to push some refs to 'http://127.0.0.1:3000/diffusion/FUN/funtudy.git'
Comment Actions
Like this feature. Any more process on this?
Or can phabricator server-side auto-merge like gerrit?
So I can just
git push origin HEAD:review
then if the Diff is accepted, it will auto-merged into repo. I have no need to merge and push it myself
| src/applications/diffusion/engine/DiffusionCommitHookEngine.php | ||
|---|---|---|
| 722 | This applies cleanly to a recent version of stable, however, there is the bug mentioned above by @liudangyi. But that one is easy enough to resolve: return DifferentialDiff::newFromRawChanges($this->getViewer(), $changes); I wonder if there is a way to maintain this as some sort of extension if the upstream is not interested in merging it? | |
Comment Actions
You could implement it as a Git pre-receive hook, I think, but that might be a fair amount of work.
There aren't really any reasonable extension points on this pathway.
We're inching closer to proxying the Git protocol and being able to bring this upstream, but it's still probably not going to happen for a while. (And this has way too many rough edges for me to be comfortable bringing it upstream without a lot more work.)
| src/applications/diffusion/engine/DiffusionCommitHookEngine.php | ||
|---|---|---|
| 147 | "fom": typo | |