Page MenuHomePhabricator

Make the default behavior of getApplicationTransactionCommentObject() "return null" instead of "throw"

Authored by epriestley on Feb 7 2019, 3:43 AM.



Depends on D20115. See

Currently, getApplicationTransactionCommentObject() throws by default. Subclasses must override it to return null to indicate that they don't support comments.

This is silly, and leads to a bunch of code that does a try / catch around it, and at least some code (here, which doesn't try / catch and gets the wrong behavior as a result.

Just make it return null by default, meaning "no support for comments". Then remove the try / catch stuff and all the return null implementations.

Test Plan
  • Grepped for getApplicationTransactionCommentObject(), fixed each callsite / definition.
  • Called on a diff with transactions (i.e., not a sourced-from-commit diff).

Diff Detail

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Feb 7 2019, 3:43 AM
epriestley added inline comments.Feb 7 2019, 3:44 AM

This is the culprit responsible for this mess.

epriestley requested review of this revision.Feb 7 2019, 3:45 AM
amckinley accepted this revision.Feb 7 2019, 8:47 PM
This revision is now accepted and ready to land.Feb 7 2019, 8:47 PM
This revision was automatically updated to reflect the committed changes.