Sam is working on new backend architecture, creating new separate classes, in T160481: Rewrite Edit Counter with new backend architecture.
This ticket is for hooking up each tool with the new architecture, once it's done.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Matthewrbowker | T153112 Epic: Rewriting XTools | |||
| Resolved | Samwilson | T165400 Write unit tests for Xtools | |||
| Resolved | Samwilson | T165399 Update all of the Xtools with new backend |
I think this is pretty much done. I mean, there are still lots of SQL queries in controllers (and missing tests), but everywhere is at least using the Project and User classes to get those things.
We do want to work towards full test coverage, but I suspect we're happy with the way things are if they're working as-is. Is that correct?
I had thought that perhaps we should work towards removing LabsHelper, but that can happen over time (it's now only got a few little methods in it anyway).
I can attest to Sam's claims. There's more cleanup to be done but we've come a long way and things are more testable. Now, on to writing those tests! =P
None of this has to be a blocker for a beta deployment, methinks. We expect an influx of bug reports and all along the while we can work to improve code quality and test coverage. Speaking for myself, this may leak into 10% time but it will be done!
Yes, let's commit to writing tests for each bug that gets raised from now on (as part of T165400: Write unit tests for Xtools).
@Samwilson: Would it be possible to post links to the relevant patches in GitHub or is it spread over too many?
@kaldari: No, I think it's too many and would be interleaved with other stuff. Probably the best thing, to get an idea of where we're at, is to look at the Controller classes (except from Default and Api, as they're different) and at how they're working with Project and User classes.
Looks sane. The directory structure is a bit complicated/confusing, especially the fact that we have a directory called Xtools in the src directory. Shouldn't we just put those files directly in the src directory (and maybe move all the stuff in the AppBundle directory up too)? It's kind of hard to find where things live currently. But maybe I'm just not groking the logic of the directory organization.
The Xtools/AppBundle differentiation is about XTools-only vs Symfony-specific classes. Not that it's all that strickt, so as you say they could probably be merged (and the former split into Model and Data or something). That'd also get rid of the current Composer warning that "defining autoload.psr-4 with an empty namespace prefix is a bad idea for performance". I'm not sure it's really worth worrying about at this stage though.