Page MenuHomePhabricator

Remove Herald rule edit log
ClosedPublic

Authored by epriestley on Apr 10 2015, 5:33 PM.
Tags
None
Referenced Files
F13287699: D12346.diff
Tue, Jun 4, 9:09 AM
F13279647: D12346.id.diff
Sat, Jun 1, 10:10 PM
F13275278: D12346.diff
Fri, May 31, 4:36 AM
F13262137: D12346.diff
Mon, May 27, 1:49 AM
F13255475: D12346.id29669.diff
Sat, May 25, 5:33 AM
F13247142: D12346.diff
Thu, May 23, 4:16 PM
F13201798: D12346.id29662.diff
Tue, May 14, 4:29 PM
F13188100: D12346.diff
Sat, May 11, 5:02 AM

Details

Summary

Fixes T7601. Ref T7803, weakly (this removes a Query subclass with ad-hoc paging). Herald has a very old edit log which predates transactions and is essentially useless and not really policy-aware. I think it's doing more harm than good; remove it.

Herald rules have proper transactions, but rule edits don't currently render something nice into the transaction log. This is definitely the way forward, but we haven't seen requests for this so don't bother building it for now.

I did put a nice end-cap on the transaction log, though.

Test Plan
  • Viewed Herald UI.
  • Grepped for removed classes and methods.
  • Edited a rule.
  • Viewed rule transaction log.

Diff Detail

Repository
rP Phabricator
Branch
hedit1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 5226
Build 5244: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Remove Herald rule edit log.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
src/applications/herald/query/HeraldEditLogQuery.php
44–46

Removing this is my tenuous connection to T7803.

btrahan edited edge metadata.
This revision is now accepted and ready to land.Apr 10 2015, 6:02 PM

FWIW The log has solved at least one 'what changed when' mystery for us.

This revision was automatically updated to reflect the committed changes.