Page MenuHomeDevCentral

Refactor analyzer classes to offer better modularity
ClosedPublic

Authored by dereckson on Aug 23 2016, 14:43.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dereckson updated this revision to Diff 1565.Aug 23 2016, 14:43
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 planned changes to this revision.Aug 23 2016, 15:33
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 updated this revision to Diff 1567.Aug 23 2016, 16:25
dereckson edited edge metadata.

Phabricator joins the show.

dereckson updated this revision to Diff 1568.Aug 23 2016, 17:07

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 updated this object.Aug 23 2016, 17:08
dereckson edited the test plan for this revision. (Show Details)
dereckson updated this object.
dereckson updated this revision to Diff 1569.Aug 23 2016, 19:26
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.