Page MenuHomePhabricator
Create Task
Maniphest T6510

Add "diffusion.didParseCommit" event
Closed, WontfixPublic
Actions

Description

Currently there is only one commit-related event called diffusion.didDiscoverCommit, which is fired after commit record is inserted to database. Unfortunately at that time the commit isn't parsed yet so if any custom code wants to look upon files, changed in that commit it can't.

I propose to add diffusion.didParseCommit event, that will be fired at the end of PhabricatorRepositoryCommitParserWorker::doWork, where we know at 100% that all commit data is available.

I suppose not much coding needs to be done (define event constant in one place and fire the event in another place), so I can do implement it as well.

I suppose the https://secure.phabricator.com/book/phabricator/article/events/ needs to be updated as well.

Related Objects

Event Timeline

aik099 created this task.Nov 9 2014, 2:51 PM
aik099 raised the priority of this task from to Needs Triage.
aik099 updated the task description. (Show Details)
aik099 added projects: Diffusion, Repositories.
aik099 added a subscriber: aik099.
aik099 updated the task description. (Show Details)Nov 9 2014, 2:59 PM
epriestley closed this task as Wontfix.Nov 9 2014, 5:02 PM
epriestley claimed this task.

We don't plan to implement this. We generally intend to move away from events: they're too complex for most users and we've found better approaches in most cases for most reasonable use cases.

aik099 added a comment.Nov 9 2014, 5:26 PM
  1. Before you've replied I've tried to implement this locally and I was very surprised when all demons at once picked up the work and PhabricatorRepositoryCommitParserWorker::doWork method was executed 5 times. Is this normal?
  2. There are no protection inside to prevent such things from happening, like daemon who wants to parse the commit locks that commit, which should prevent others from doing the same in parallel.
  3. Is there other (non-event based) way to do custom code after commit is parsed? If so, then where this code should be placed?
aik099 added a comment.Nov 9 2014, 5:36 PM

Before you've replied I've tried to implement this locally and I was very surprised when all demons at once picked up the work and PhabricatorRepositoryCommitParserWorker::doWork method was executed 5 times. Is this normal?

I've figured out what's happened. The PhabricatorRepositoryCommitParserWorker is subclassed by :

  • PhabricatorRepositoryCommitHeraldWorker
  • PhabricatorRepositoryCommitOwnersWorker
  • PhabricatorRepositoryCommitChangeParserWorker
  • PhabricatorRepositoryCommitMessageParserWorker

And since I've logged parse event in parent class it was executed for all of them.

Is there a centralized way to determine when comment was completely parsed by whoever wanted to parse it?

aik099 added a comment.Nov 11 2014, 8:33 PM

I've got desired effect by adding my code into custom Harbormaster step, that I've attached through Herald to be executed on new commits, see T6518.

Phabricator · Built by Phacility