Page MenuHomeDevCentral
Differential D274

Refactor GitHubPayloadAnalyzer into payloads individual rules
ClosedPublic
Actions

Authored by dereckson on Feb 6 2016, 05:36.
Tags
None
Referenced Files
F3920273: D274.diff
Fri, Dec 20, 14:09
Unknown Object (File)
Sat, Dec 14, 19:15
Unknown Object (File)
Fri, Dec 13, 06:12
Unknown Object (File)
Fri, Dec 13, 05:20
Unknown Object (File)
Thu, Dec 12, 17:26
Unknown Object (File)
Thu, Dec 12, 06:56
Unknown Object (File)
Thu, Dec 12, 05:13
Unknown Object (File)
Tue, Dec 10, 21:19
View All Files
Subscribers
alken-orin

Details

Reviewers
dereckson
Maniphest Tasks
T704: Split GitHubPayloadAnalyzer::getDescription in classes
Commits
rNOTIF98e21dc8f292: Refactor GitHubPayloadAnalyzer into payloads individual rules
Summary

Instead of trying to parse every payload in GitHubPayloadAnalyzer,
we split the processing into several rules class, one class per type.
They are located in app/Analyzers/GitHub/Events.

So, if GitHub introduces a new event type, the only thing to do is
to create a new class for this event, provide sensible return values
for getDescription and getLink. The class will then be autoloaded.

Test Plan

phpunit

Code coverage at 100% for the new classes and the
modified parts of GitHubPayloadAnalyzer.

Diff Detail

Repository
rNOTIF Notifications center
Lint
Lint Passed
Unit
Tests Passed
Branch
refactoring/GitHubPayloadAnalyzer

Event Timeline

dereckson updated this revision to Diff 635.Feb 6 2016, 05:36
dereckson retitled this revision from to [WIP] Refactor GitHubPayloadAnalyzer.
dereckson updated this object.
dereckson edited the test plan for this revision. (Show Details)
dereckson added a reviewer: dereckson.
alken-orin added a subscriber: alken-orin.Feb 6 2016, 05:36

Build started: https://ci.nasqueron.org/job/test-notifications-phab/57/ (console: https://ci.nasqueron.org/job/test-notifications-phab/57/console)

dereckson updated this revision to Diff 636.Feb 6 2016, 05:41
dereckson edited edge metadata.

Whitespace issues

alken-orin added a comment.Feb 6 2016, 05:42

Build is green https://ci.nasqueron.org/job/test-notifications-phab/57/ for more details.

alken-orin added a comment.Feb 6 2016, 05:42

Build started: https://ci.nasqueron.org/job/test-notifications-phab/58/ (console: https://ci.nasqueron.org/job/test-notifications-phab/58/console)

alken-orin added a comment.Feb 6 2016, 05:47

Build is green https://ci.nasqueron.org/job/test-notifications-phab/58/ for more details.

dereckson updated this revision to Diff 637.Feb 8 2016, 17:22
dereckson edited edge metadata.

More tests: WithCommit, DeleteEvent

alken-orin added a comment.Feb 8 2016, 17:22

Build started: https://ci.nasqueron.org/job/test-notifications-phab/59/ (console: https://ci.nasqueron.org/job/test-notifications-phab/59/console)

alken-orin added a comment.Feb 8 2016, 17:27

Build is green https://ci.nasqueron.org/job/test-notifications-phab/59/ for more details.

dereckson updated this revision to Diff 638.Feb 8 2016, 20:04

UnknownEvent

alken-orin added a comment.Feb 8 2016, 20:04

Build started: https://ci.nasqueron.org/job/test-notifications-phab/60/ (console: https://ci.nasqueron.org/job/test-notifications-phab/60/console)

alken-orin added a comment.Feb 8 2016, 20:09

Build is green https://ci.nasqueron.org/job/test-notifications-phab/60/ for more details.

dereckson updated this revision to Diff 639.Feb 8 2016, 22:11

Previous diff 638 overwrote the UnknownEvent class with the test class.

alken-orin added a comment.Feb 8 2016, 22:11

Build started: https://ci.nasqueron.org/job/test-notifications-phab/61/ (console: https://ci.nasqueron.org/job/test-notifications-phab/61/console)

alken-orin added a comment.Feb 8 2016, 22:16

Build is green https://ci.nasqueron.org/job/test-notifications-phab/61/ for more details.

dereckson updated this revision to Diff 640.Feb 9 2016, 01:59
  • More tests
  • Fixed a l10n issue in PushEvent::getDescriptionMessageKey
alken-orin added a comment.Feb 9 2016, 01:59

Build started: https://ci.nasqueron.org/job/test-notifications-phab/62/ (console: https://ci.nasqueron.org/job/test-notifications-phab/62/console)

alken-orin added a comment.Feb 9 2016, 02:04

Build is green https://ci.nasqueron.org/job/test-notifications-phab/62/ for more details.

dereckson updated this revision to Diff 641.Feb 9 2016, 02:24

+StatusEventTest

alken-orin added a comment.Feb 9 2016, 02:25

Build started: https://ci.nasqueron.org/job/test-notifications-phab/63/ (console: https://ci.nasqueron.org/job/test-notifications-phab/63/console)

alken-orin added a comment.Feb 9 2016, 02:30

Build is green https://ci.nasqueron.org/job/test-notifications-phab/63/ for more details.

dereckson updated this revision to Diff 642.Feb 9 2016, 03:52

StatusEventTest

alken-orin added a comment.Feb 9 2016, 03:52

Build started: https://ci.nasqueron.org/job/test-notifications-phab/64/ (console: https://ci.nasqueron.org/job/test-notifications-phab/64/console)

alken-orin added a comment.Feb 9 2016, 03:57

Build is green https://ci.nasqueron.org/job/test-notifications-phab/64/ for more details.

dereckson updated this revision to Diff 643.Feb 9 2016, 16:01

RepositoryEventTest coverage

alken-orin added a comment.Feb 9 2016, 16:01

Build started: https://ci.nasqueron.org/job/test-notifications-phab/65/ (console: https://ci.nasqueron.org/job/test-notifications-phab/65/console)

alken-orin added a comment.Feb 9 2016, 16:06

Build is green https://ci.nasqueron.org/job/test-notifications-phab/65/ for more details.

dereckson updated this revision to Diff 644.Feb 9 2016, 16:46
  • GitHubPayloadAnalyzerTest code coverage for our refactoring
  • Tidy up a little these files
alken-orin added a comment.Feb 9 2016, 16:46

Build started: https://ci.nasqueron.org/job/test-notifications-phab/66/ (console: https://ci.nasqueron.org/job/test-notifications-phab/66/console)

alken-orin added a comment.Feb 9 2016, 16:51

Build is green https://ci.nasqueron.org/job/test-notifications-phab/66/ for more details.

dereckson retitled this revision from [WIP] Refactor GitHubPayloadAnalyzer to Refactor GitHubPayloadAnalyzer into payloads individual rules .Feb 9 2016, 16:52
dereckson updated this object.
dereckson edited the test plan for this revision. (Show Details)
dereckson updated this object.
dereckson updated this revision to Diff 645.Feb 9 2016, 17:08
dereckson retitled this revision from Refactor GitHubPayloadAnalyzer into payloads individual rules to Refactor GitHubPayloadAnalyzer into payloads individual rules.
dereckson updated this object.

Final tweaks, renamed DockerHubPush… by GitHubPush…

alken-orin added a comment.Feb 9 2016, 17:08

Build started: https://ci.nasqueron.org/job/test-notifications-phab/67/ (console: https://ci.nasqueron.org/job/test-notifications-phab/67/console)

alken-orin added a comment.Feb 9 2016, 17:13

Build is green https://ci.nasqueron.org/job/test-notifications-phab/67/ for more details.

dereckson accepted this revision.Feb 9 2016, 17:18
dereckson edited edge metadata.
This revision is now accepted and ready to land.Feb 9 2016, 17:18
Closed by commit rNOTIF98e21dc8f292: Refactor GitHubPayloadAnalyzer into payloads individual rules (authored by dereckson, committed by dereckson). · Explain WhyFeb 9 2016, 17:19
This revision was automatically updated to reflect the committed changes.
dereckson added a task: T704: Split GitHubPayloadAnalyzer::getDescription in classes.Feb 9 2016, 18:50

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
app/
Analyzers/
GitHub/
Events/
38 lines--
77 lines72%
Loading...
73 lines71%
Loading...
78 lines100%
Loading...
35 lines--
67 lines37%
Loading...
42 lines--
74 lines--
29 lines--
64 lines--
43 lines--
51 lines--
186 lines--
resources/
lang/
en/
53 lines--
tests/
Analyzers/
GitHub/
Events/
41 lines--
41 lines--
88 lines--
36 lines--
29 lines--
data/
payloads/
GitHubEvents/
140 lines--
113 lines--
111 lines--
161 lines--
119 lines--
206 lines--

Diff 638

View Options

app/Analyzers/GitHub/Events/CommitCommentEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/CreateEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/DeleteEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/Event.php

Loading...
View Options

app/Analyzers/GitHub/Events/PingEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/PushEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/RepositoryEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/StatusEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/UnknownEvent.php

Loading...
View Options

app/Analyzers/GitHub/Events/WithCommit.php

Loading...
View Options

app/Analyzers/GitHub/Events/WithRef.php

Loading...
View Options

app/Analyzers/GitHub/Events/WithRepoAndBranch.php

Loading...
View Options

app/Analyzers/GitHub/GitHubPayloadAnalyzer.php

Loading...
View Options

resources/lang/en/GitHub.php

Loading...
View Options

tests/Analyzers/GitHub/Events/CreateEventTest.php

Loading...
View Options

tests/Analyzers/GitHub/Events/DeleteEventTest.php

Loading...
View Options

tests/Analyzers/GitHub/Events/EventTest.php

Loading...
View Options

tests/Analyzers/GitHub/Events/PushEventTest.php

Loading...
View Options

tests/Analyzers/GitHubPayloadAnalyzerTest.php

Loading...
View Options

tests/data/payloads/GitHubEvents/commit_comment.json

Loading...
View Options

tests/data/payloads/GitHubEvents/create.json

Loading...
View Options

tests/data/payloads/GitHubEvents/delete.json

Loading...
View Options

tests/data/payloads/GitHubEvents/push.json

Loading...
View Options

tests/data/payloads/GitHubEvents/repository.json

Loading...
View Options

tests/data/payloads/GitHubEvents/status.json

Loading...
Nasqueron DevCentral · If it had been much bigger the moon would have had a core of ice. · Powered by Phabricator