Ref T1266. This doesn't change any behaviors, but some of this code has a lot of really complicated conditionals and I tried to break that up a bit.
Also, reexpress this stuff in terms of the "structured" parser in D12144.
Details
Details
- Reviewers
btrahan - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rP373aaa643a51: Clean up copy detection code a bit
Unit tests still pass. They aren't hugely comprehensive but did reliably fail when I screwed stuff up.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| src/applications/differential/parser/DifferentialChangesetParser.php | ||
|---|---|---|
| 1347 | In particular, this kind of stuff feels sketchy and unpredictable to me and I wanted to get rid of it before changing behavior. | |
Comment Actions
Might be worth a quick comparison of a revision that has copy detection / holding this a bit 'til subsequent diff(s) where you do that sort of testing or unit tests are more comprehensive, etc... Anyway, much better code on the right hand side!
| src/applications/differential/parser/DifferentialChangesetParser.php | ||
|---|---|---|
| 1347 | good lord | |
Comment Actions
Yeah, D12146 adds more coverage and I looked through ~5 revisions after that to see if I could spot anything obvious. The new behavior after that change generally seems better, but yell if you spot anything sketchy looking after that pushes.