Skip to content

refactor(vcs): report the events a delivery describes - #128

Merged
HarshMN2345 merged 2 commits into
mainfrom
feat-get-events
Aug 7, 2026
Merged

refactor(vcs): report the events a delivery describes#128
HarshMN2345 merged 2 commits into
mainfrom
feat-get-events

Conversation

@HarshMN2345

Copy link
Copy Markdown
Member

Splits the webhook-parsing change out of #125 so that PR carries only Bitbucket.

What changes

getEvent() becomes getEvents() and returns the events a delivery describes rather than a single event. getEvent() no longer exists.

A delivery was read as one event, which holds for every provider on main but not for all of them: a Bitbucket push reports each ref it touched in one delivery, and only the first would ever have been seen.

Shape

Providers here carry one event per delivery, so each returns the one it parsed inside a list. A delivery describing nothing still returns an empty list, which now reads as "no events" rather than "one empty event".

before after
known event ['branch' => 'main', ...] [['branch' => 'main', ...]]
unsupported event [] []
invalid payload throws throws

Follow-ups

  • Appwrite consumes the result as a list and loops it.
  • Add Bitbucket adapter #125 then drops the wrapper it currently adds and implements getEvents() like the rest.

Green on all six adapters.

A delivery was read as one event, which holds for every provider here but
not for all of them: a Bitbucket push reports each ref it touched in a
single delivery, and only the first would have been seen.

getEvents() replaces getEvent() and hands back the events a delivery
describes, empty when it describes none. Providers that carry one event per
delivery return the one they parsed, so what callers read is unchanged
beyond the list around it.
Comment thread tests/VCS/Adapter/GitHubTest.php Outdated
}

$result = $this->vcsAdapter->getEvent('installation', $payload);
$result = $this->vcsAdapter->getEvents('installation', $payload)[0];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all palces that we did this, lets also assertIsArray and assertCount 1

Reading the first event said nothing about how many came back, so a
delivery parsed into none or several would have failed on the event's
contents rather than on the count that explains it.
@HarshMN2345
HarshMN2345 merged commit 0497be6 into main Aug 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants