...and get the basic edit flow "working" for a new NuanceSourceDefinition - the Phabricator Form. ...and fix a dumb bug in the query class so when you redirect to the view page / try to edit an existing NuanceSource you don't fatal.
Details
Details
- Reviewers
epriestley - Commits
- Restricted Diffusion Commit
rP7b718bb033a3: Nuance - federate out the design of NuanceSource via NuanceSourceDefinition
played around with the edit form and it worked!
Diff Detail
Diff Detail
- Branch
- n3
- Lint
Lint Passed Severity Location Code Message Advice src/applications/nuance/source/NuancePhabricatorFormSourceDefinition.php:22 XHP16 TODO Comment Advice src/applications/nuance/source/NuancePhabricatorFormSourceDefinition.php:32 XHP16 TODO Comment - Unit
Tests Passed
Event Timeline
Comment Actions
Minor stuff inline.
| src/applications/nuance/editor/NuanceSourceEditor.php | ||
|---|---|---|
| 9–10 | Consider just rendering TYPE_NAME as "alincoln created this source." if the old value is empty, instead of having a separate CREATE transaction. | |
| src/applications/nuance/query/NuanceSourceQuery.php | ||
| 38 | haha oops | |
| src/applications/nuance/source/NuanceSourceDefinition.php | ||
| 96–101 | Maybe consider strings instead of numbers for the constants, we'll probably never change it if we don't now. | |
| 104 | "updat" typo | |
| 112 | I don't understand what this method does from the comment. I'd expect scour() to be like a delete/wipe/reset of some kind. Is this scour like "search", not scour like "clean"? Maybe scourForNewItems or something? | |
| 114 | Consider load vs get for explictness. | |
| 227 | In theory we can do validation in the Editor now, although that might be more work. | |
Comment Actions
- change source type from int to string
- address other feedback too
- add some code to the view controller so you can see this stuff just a wee bit more