Page MenuHomePhabricator
Differential D17848

Clean up some rough Macro transaction edges
ClosedPublic
Actions

Authored by chad on May 8 2017, 8:36 PM.
Tags
None
Referenced Files
F13190375: D17848.id42928.diff
Sat, May 11, 10:18 AM
F13184538: D17848.id42929.diff
Fri, May 10, 8:07 PM
F13182598: D17848.id42941.diff
Thu, May 9, 11:57 PM
F13175276: D17848.id42942.diff
Wed, May 8, 4:24 AM
Unknown Object (File)
Tue, May 7, 2:16 PM
Unknown Object (File)
Tue, May 7, 2:03 PM
Unknown Object (File)
Fri, May 3, 5:00 AM
Unknown Object (File)
Sun, Apr 28, 4:57 AM
View All Files
Subscribers
Korvin

Details

Reviewers
epriestley
Maniphest Tasks
T12685: Modular Transactions Migrations Errata
Commits
rP3dae9701298f: Clean up some rough Macro transaction edges
Summary

Ref T12685, cleans up various macro issues, remove subscribers, fix feed stories, etc.

Test Plan

Create a new macro, see no subscribers, edit various macros.

Diff Detail

Repository
rP Phabricator
Branch
macro-xaction-3 (branched from master)
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/applications/macro/xaction/PhabricatorMacroFileTransaction.php:65XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 16854
Build 22502: Run Core Tests
Build 22501: arc lint + arc unit

Event Timeline

chad created this revision.May 8 2017, 8:36 PM
Herald added a subscriber: Korvin. · View Herald TranscriptMay 8 2017, 8:36 PM
Harbormaster completed remote builds in B16854: Diff 42928.May 8 2017, 8:37 PM
chad updated this revision to Diff 42929.May 8 2017, 8:42 PM
  • fix edit with no macro?
Harbormaster completed remote builds in B16855: Diff 42929.May 8 2017, 8:43 PM
epriestley edited edge metadata.May 8 2017, 8:44 PM

One inline I'm not sure about, looks good otherwise.

src/applications/macro/editor/PhabricatorMacroEditEngine.php
75–76

GREAT comment A+++

src/applications/macro/xaction/PhabricatorMacroFileTransaction.php
13–14

What's this one for?

chad added inline comments.May 8 2017, 8:46 PM
src/applications/macro/xaction/PhabricatorMacroFileTransaction.php
13–14

Editing a macro without changing the file generates an empty transaction instead of no transaction.

13–14

Specifically, it tries to set null, and the column isn't nullable.

epriestley added a comment.May 8 2017, 11:01 PM

Let me see if I can make FileEditField or whatever it is a little smarter about things.

chad added a comment.May 8 2017, 11:13 PM

isorry

epriestley mentioned this in D17853: Improve the behavior of PhabricatorFileEditField for Macros.May 8 2017, 11:18 PM
epriestley added a comment.May 8 2017, 11:19 PM

If you put this on top of D17853, I think you can delete that if ($value) { bit and everything will just work better (not perfectly, but better).

epriestley mentioned this in rPa1f5d37357d6: Improve the behavior of PhabricatorFileEditField for Macros.May 8 2017, 11:23 PM
chad updated this revision to Diff 42941.May 8 2017, 11:27 PM
  • works betters
Harbormaster completed remote builds in B16867: Diff 42941.May 8 2017, 11:28 PM
epriestley accepted this revision.May 8 2017, 11:28 PM
This revision is now accepted and ready to land.May 8 2017, 11:28 PM
Closed by commit rP3dae9701298f: Clean up some rough Macro transaction edges (authored by chad, committed by chad). · Explain WhyMay 8 2017, 11:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
applications/
macro/
editor/
22 lines
xaction/
37 lines
3 lines
subscriptions/
engineextension/
3 lines

Diff 42928

View Options

src/applications/macro/editor/PhabricatorMacroEditEngine.php

Loading...
View Options

src/applications/macro/xaction/PhabricatorMacroFileTransaction.php

Loading...
View Options

src/applications/macro/xaction/PhabricatorMacroNameTransaction.php

Loading...
View Options

src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php

Loading...
Phabricator · Built by Phacility