Page MenuHomePhabricator

gerritbot's comments no longer add the #Patch-For-Review project
Closed, ResolvedPublic

Event Timeline

matmarex created this task.Feb 19 2015, 4:53 PM
matmarex raised the priority of this task from to High.
matmarex updated the task description. (Show Details)
matmarex added a subscriber: matmarex.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 19 2015, 4:53 PM

Argh. Thanks for catching this!
Since T86772, mentioning a project in a comment does not automatically add it to the associated projects of a task anymore, and I wasn't aware that gerritbot only adds a comment mentioning that project and does not really "associate" a project.

Project phid of Patch-For-Review in Wikimedia Phabricator seems to be

PHID-PROJ-onnxucoedheq3jevknyr

according to project.query; should be passed as phid in maniphest.update ?

Downstream code is at https://gerrit.wikimedia.org/r/#/admin/projects/operations/gerrit/plugins
Upstream project is at https://gerrit-review.googlesource.com/#/admin/projects/plugins/its-phabricator

Aklapper moved this task from To Triage to Ready to Go on the Phabricator board.Feb 19 2015, 7:29 PM

Looked a bit at the its-base source code in ./src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/ActionExecutor.java and Wikimedia puppet's modules/gerrit/files/its/ and it looked easier to create a Herald rule (add Patch-For-Review project to ask when comment created by Gerritbot and comment contains Patch-For-Review) for the time being: H23.

Keeping this open as I want to check if this works.

Aklapper moved this task from Ready to Go to Doing on the Phabricator board.Feb 20 2015, 12:22 PM

Nope,

Body | is | #patch-for-review
Author | is any of | gerritbot

didn't work as seen in T87832. Because "Author" probably refered to initial task desc. Now trying with

CCs | include all of | gerritbot

though not convinced yet either if that'll trigger.

Not working. So I tried with "Body | contains | Patch-For-Review". Still not working (watching the feed: 90245; Feb 20, 7:01 PM), and checking the code in /src/applications/herald/adapter/HeraldManiphestTaskAdapter.php it only looks at the description:

case self::FIELD_BODY:
  return $this->getTask()->getDescription();

So it looks like we really have to fix this in its-phabricator instead and Herald cannot help. :(

Restricted Application added a project: Patch-For-Review. · View Herald TranscriptFeb 20 2015, 7:46 PM
Aklapper set Security to None.

Wondering if there is something like "action = project Patch-For-Review" to just add to puppet's modules/gerrit/files/its/action.config. Which requires some "project" action handler. /me getting lost in code. :-/

Se4598 added a subscriber: Se4598.Feb 20 2015, 9:14 PM

Once again a backwards incompatible update of the issue tracker without testing if bots still work? Ouch :-(

If no one comes up with a fix before that, I can start working on a fix on Wednesday.

Aklapper reassigned this task from Aklapper to QChris.Feb 23 2015, 6:11 PM

Change 192613 had a related patch set uploaded (by QChris):
Upgrade phabricator plugins to add add-project action

https://gerrit.wikimedia.org/r/192613

Patch-For-Review

Change 192624 had a related patch set uploaded (by QChris):
Explicitly associate 'Patch-For-Review' project to tasks

https://gerrit.wikimedia.org/r/192624

Patch-For-Review

Change 192624 merged by Rush:
Explicitly associate 'Patch-For-Review' project to tasks

https://gerrit.wikimedia.org/r/192624

QChris closed this task as Resolved.Feb 24 2015, 8:54 PM

The bot again adds the Patch-For-Review project: https://phabricator.wikimedia.org/T775#1063782

Change 192613 merged by Chad:
Upgrade phabricator plugins to add add-project action

https://gerrit.wikimedia.org/r/192613

oh the lovely times from bugzilla's gerritbot are back: one email for the comment, another one for the tag...

oh the lovely times from bugzilla's gerritbot are back: one email for the comment, another one for the tag...

Correct. Patches to rewrite the code welcome.

Danny_B edited projects, added GerritBot; removed Gerrit.May 19 2016, 3:00 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMay 19 2016, 3:00 PM