Uh oh!
There was an error while loading. Please reload this page.
refactor: introduced Diff and FileChange VOs - #29
Conversation
NickSdot
commented
Jul 20, 2026
@jordikroon I had my first risky coverage CI failure here. Adding every class as an attribute, plus having its import, seems quite excessive. 😅 Are we fine with grouped attributes + FQN? #[UsesClass(\DocbookCS\Source\File::class), UsesClass(\DocbookCS\Source\Line::class)]
finalclass SniffRunnerTest extends TestCase {
/// ...
} |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jordikroon
commented
Jul 20, 2026
It is excessive and long, however I assume there is a high chance the class is used in the test directly. For that reason it's better to not use the FQNS. Otherwise a portion will be inconstant. Some IDE's do collapse the list of imported namespaces though. |
7b29346 to
bdcf4e2CompareNickSdot
commented
Jul 21, 2026
jordikroon
commented
Jul 21, 2026
Thanks. And as discussed you will handle the feedback in follow up MRs. |
Uh oh!
There was an error while loading. Please reload this page.
NickSdot
commented
Jul 21, 2026
Yups. Both should be handled/obsolete. If I missed something let me know. Will rebase now, then un-draft the next one. |
Preflight. Adds two DTOs and extracts some logic from the sniffer. Not working with arrays made things much easier. Test are so that we have as little diff churn as possible and the assertions are goal-wise the same.
DiffParserTest has a small helper addition
lineNumbersByFile()which I marked myself as todo; not optimal to merge todos but I need to keep things a bit organised if I want to get the fixer PR diff smaller. When fixers are merged I will follow up with the various todos.