Page MenuHomePhabricator

phabstatus: clean up showsyncstatus()
ClosedPublic

Authored by simpkins on Jul 13 2017, 10:29 PM.

Details

Reviewers
ryanmce
Group Reviewers
Restricted Project
Commits
rFBHGXb2de23e301d2: phabstatus: clean up showsyncstatus()
Summary

Refactor the showsyncstatus function to clean up the code a little bit.

  • Don't bother to call populateresponseforphab() if the current commit does not contain a differential revision ID.
  • Only call getdiffstatus() once instead of 4 times. getdiffstatus() does cache the result instead of hitting phabricator each time, but it is still better to just re-use the return value ourselves instead of hitting the memoization code.
  • Refactor the error handling to avoid having deeply nested conditional blocks.
Test Plan

Confirmed existing tests still pass, and manually tested using the template.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Branch
default
Lint
Lint OK
Unit
Unit Tests OK

Event Timeline

simpkins created this revision.Jul 13 2017, 10:29 PM
ryanmce accepted this revision.EditedJul 19 2017, 9:54 AM
ryanmce added a subscriber: ryanmce.
This revision is now accepted and ready to land.Jul 19 2017, 9:54 AM
This revision was automatically updated to reflect the committed changes.