Page MenuHomePhabricator

Update all of the Xtools with new backend
Closed, ResolvedPublic8 Estimated Story Points

Description

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.

  • Edit Counter
  • Article Info ("Page History")
  • Pages Created
  • Top Edits
  • AdminStats
  • RfX Analysis
  • RfX Vote Counter
  • Quick, Dirty, Simple Edit Counter
  • Automated Edits

Event Timeline

DannyH created this task.May 15 2017, 9:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 15 2017, 9:06 PM
DannyH updated the task description. (Show Details)May 15 2017, 11:07 PM
DannyH triaged this task as Medium priority.May 16 2017, 9:51 PM
Samwilson updated the task description. (Show Details)May 16 2017, 11:15 PM
Samwilson updated the task description. (Show Details)
kaldari set the point value for this task to 8.May 16 2017, 11:16 PM
Matthewrbowker moved this task from Inbox to Working on the XTools board.May 19 2017, 12:04 AM

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.

kaldari closed this task as Resolved.Jun 27 2017, 2:09 AM
kaldari moved this task from Needs Review/Feedback to Q1 2018-19 on the Community-Tech-Sprint board.
DannyH moved this task from Estimated to Archive on the Community-Tech board.Jul 4 2017, 12:53 AM
MusikAnimal moved this task from Working to Complete on the XTools board.Jul 23 2017, 7:35 PM