Page MenuHomeDevCentral

Refactor analyzer classes to offer better modularity
ClosedPublic

Authored by dereckson on Aug 23 2016, 14:43.
Tags
None
Referenced Files
F3717177: D629.id1567.diff
Wed, Nov 6, 01:45
F3717042: D629.id1570.diff
Tue, Nov 5, 23:51
F3715409: D629.id1567.diff
Tue, Nov 5, 05:56
Unknown Object (File)
Tue, Oct 29, 16:55
Unknown Object (File)
Mon, Oct 28, 12:23
Unknown Object (File)
Mon, Oct 28, 12:23
Unknown Object (File)
Mon, Oct 28, 12:23
Unknown Object (File)
Mon, Oct 28, 12:22
Subscribers

Details

Summary

The GitHub and Jenkins analyzers configuration and workflows
are very similar: they map items (repos or jobs) to groups.

Phabricator is more complex, as we want to try to sort what humans
forgot to sort (e.g. a task without any project but with a keyword
in the title can go to the right group), but still shares the same
logic to locate or parse the configuration file.

The analyzer configuration for GitHub so becomes the base class
PayloadAnalyzerConfiguration. It can be used as is if nothing
else is to be configured than an item/group mapping.

When something else is needed (replace mapping for Phabricator,
add notify only on failure array for Jenkins), we extend it.

The base analyzer will load only the custom class if existing,
and will use the base class when it doesn't.

That simplify a lot the preparation of an analyzer for a new service:
if only mapping is needed, only one analyzer class is needed, and they
can reuse standard payload analyzer configuration.

Test Plan
  • Current unit tests already cover abstract class methods, as they test the GitHub implementation.
  • Edit configurations to use 'map', run tests, fire payloads.
  • Check test coverage for base classes is well 100%.

Diff Detail

Repository
rNOTIF Notifications center
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dereckson retitled this revision from to Refactor analyze class to offer better modularity.
dereckson updated this object.
dereckson edited the test plan for this revision. (Show Details)
dereckson added a reviewer: dereckson.
dereckson retitled this revision from Refactor analyze class to offer better modularity to Refactor analyzer classes to offer better modularity.
dereckson edited edge metadata.

We could get 100% of Analyzer coverage if:

  • Phabricator uses it
  • administrave event notion is abstracted
dereckson edited edge metadata.

Phabricator joins the show.

The analyzer configuration for GitHub becomes the base class
PayloadAnalyzerConfiguration. It can be used as is if nothing
else is to be configured than an item/group mapping.

When something else is needed (replace mapping for Phabricator,
add notify only on failure array for Jenkins), we extend it.

The base analyzer will load only the custom class if existing,
and will use the base class when it doesn't.

That simplify a lot the preparation of an analyzer for a new service:
if only mapping is needed, only one analyzer class is needed, and they
can reuse standard payload analyzer configuration.

dereckson edited the test plan for this revision. (Show Details)
dereckson updated this object.
dereckson updated this object.
dereckson edited the test plan for this revision. (Show Details)

Test for BasePayloadAnalyzer::getItemName

This revision was automatically updated to reflect the committed changes.