Page MenuHomePhabricator

Development strategy for collaborating on Parsoid REST API routes as Parsoid-PHP is being developed
Closed, ResolvedPublic

Description

Parsoid-PHP is being developed as a Composer library.

It is in active development while we develop these routes.

It has not been subject to security review, so code that imports the Parsoid-PHP library can't be merged into the master branch for MediaWiki.

We have at least these options:

  1. Defer developing the Parsoid routes until the Parsoid PHP library meets the requirements necessary to merge to master. Estimate is ~2-3 months from today, at best.
  2. Create a feature branch in Git. This is unusual while we use Gerrit.
  3. Create a "main" patch in Gerrit, and then other patches depend on that main patch, so they are effectively a feature branch. Perhaps the main patch simply imports Parsoid-PHP?
  4. Dependency injection...? Or a mechanism to use a mock Parsoid PHP interface, which can be replaced with the Parsoid PHP library when that is ready to merge to master.

Event Timeline

@Tgr @tstarling @Anomie per our discussion in sprint meeting

This may be of interest to @daniel w/r/t strategies for decoupling

ssastry added a comment.EditedMay 28 2019, 12:47 PM

Defer developing the Parsoid routes until the Parsoid PHP library meets the requirements necessary to merge to master. Estimate is ~2-3 months from today, at best.

This is not a real option as far as I am concerned, i.e. there is no real reason to wait on this. If necessary, just like the config code (which actually belongs in core. but lives in Parsoid-PHP repo right now), these patches can live in the Parsoid-PHP repo temporarily during development. And the patches can be moved to core via one of strategies 2-4 when they are ready to be tested.

JTannerWMF moved this task from Inbox to External on the Growth-Team board.May 28 2019, 6:09 PM
JTannerWMF added a subscriber: JTannerWMF.

We are marking this as external, as this doesn't appear to be actionable for Growth.

The problem comes down to the fact that we can't merge code for this into MediaWiki core master due to certain blockers, so if we don't want to block development on those blockers we have to pick some way to do outside-of-master development. The tradeoffs basically come down to workflow.

  1. Commit to a branch, and have to merge it into master later.
    • Development would check it out.
    • History in the eventual merged-into-core code could wind up a bit convoluted, if things don't stay within new files. See the early history of stuff related to ContentHandler, for example.
  2. Use chained unmerged Gerrit changesets as a "branch".
    • Development would require checking all the unmerged changesets out, which might be a pain if they're not a linear series.
    • History would be straightforward, but the rebases would probably be a pain.
  3. Merge a bunch of mocks into MediaWiki core master for now, and drop them later.
    • Development would involve making these mocks work sufficiently for whatever purpose we're trying to use them for, keeping them in sync with Parsoid-PHP, and eventually deleting them.
    • Risk would be in spending time trying to make the mocks too functional rather than progressing on the real library. And in other code trying to start using the mocks before we're ready to try to define a real interface for things other than these REST endpoints.
  4. Develop the code in some other repo, and copy to MediaWiki core later.
    • Could be arbitrarily in a subdirectory of Parsoid-PHP (as suggested in T224471#5216381), or could be in a temporary extension (suggested in the Parsoid meeting today).
      • Let's put "Temporary" in the extension name, so as to not let anyone start to think it's not temporary.
    • Development would involve pulling in the code from the other repo in some manner. As an extension that would probably be extremely straightforward (we need to write that code anyway), as an arbitrary subdirectory it might be less so. OTOH, the arbitrary subdirectory could be exactly what would be in an extension anyway, just not as a separate repo, which would have the advantage of fewer cross-repo changes.
    • History might be lost (or only included by reference in the commit summary), or might be like #2, depending on how the eventual merge-into-core happens.

After writing all that, I'm liking #5 so far, specifically the version of structuring a subdirectory in the Parsoid-PHP repo as a MW extension, followed by #2. #4 seems like too much work that will be thrown away, and #3 seems like rebase hell.

The problem comes down to the fact that we can't merge code for this into MediaWiki core master due to certain blockers, so if we don't want to block development on those blockers we have to pick some way to do outside-of-master development. The tradeoffs basically come down to workflow.

I don't have a strong opinion / preference around this as long as development is not blocked. So, I'll mute myself on this task unless I see something that needs my input.

Tgr added a comment.May 28 2019, 10:40 PM

I think the original misunderstanding was around merging API stuff (ie. having it available on Beta or production) while Parsoid is not in a mergeable (to /vendor) state. If the goal is making it possible to manually set something up in a development environment, Gerrit patch trees are fine. (And IMO preferrable to feature branches as the patches remain editable. If we need the Parsoid endpoints well before the API becomes stable, that's probably helpful.)

Tgr added a comment.May 29 2019, 1:51 PM

Btw, is the plan to initially write the handlers using the stubs that are in T221988: Spec a PHP interface that will serve as an entry point into the Parsoid-PHP composer lib? Or does that task cover adding the fully functional entry points as well?

@Tgr yes, that's the plan.

EvanProdromou added a comment.EditedMay 29 2019, 6:15 PM

@Anomie One option neither of us brought up was developing the Parsoid routes as part of a MediaWiki extension. That would not only exercise the ability of the REST API Router to be useful for extensions, but it would allow us to manage the Parsoid interface without committing to master.

The only problem I can think of is that I believe eventually we'd expect the Parsoid routes to be available by default in new MediaWiki installations. Also, some of the classes, like the siteconfig and pagecontext classes, would be necessary in core once Parsoid is the default parser.

However, if we plan on that from the beginning and keep the extension-related shell to a minimum, it probably wouldn't be too hard to merge that in...?

@Anomie Somehow I managed to not read #5 very well. I think we're on the same page.

Tgr closed this task as Resolved.Jun 4 2019, 12:00 PM

I guess this is done since we agreed on T224471#5218407 option #5 for T224979: Parsoid REST Routes Extension.