Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
Status: Active » Needs review
FileSize
23.16 KB
effulgentsia’s picture

In core, we also have RenderCacheInterface, which is marked @internal, and LinkGeneratorInterface, where each of its methods is marked @internal. Are we saying, we should in principle remove those interfaces too? Not this issue's scope; just curious if the same logic behind this patch applies broadly to core.

Wim Leers’s picture

I think you're right, those had interfaces created for the sake of creating interfaces. There was a time when we were pretty much blindly extracting interfaces out of classes. Doing it blindly was probably wrong. But now we can't change that.

We can still extract an interface later — that's equivalent with adding an API. But we can never remove an API. Hence this approach makes sense.


A different angle:

  • Those are subsystems, each providing an API designed to be used by just about every Drupal module. That highly increases the likelihood of swapping/extending their implementation. Because the API may need to be bent to deal with a particular edge case.
  • BigPipe on the other hand provides no API at all. It consumes the Render API and the request processing API. The likelihood of swapping/extending its implementation is orders of magnitude lower. But it's still possible — you just have to subclass it.
Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

RTBC - I agree, interfaces just for the sake of interfaces makes no sense.

Wim Leers’s picture

Title: Remove BigPipeInterface and move all of its docs to the implementation » [PP-1] Remove BigPipeInterface and move all of its docs to the implementation
Status: Reviewed & tested by the community » Postponed
Wim Leers’s picture

Title: [PP-1] Remove BigPipeInterface and move all of its docs to the implementation » Remove BigPipeInterface and move all of its docs to the implementation
Status: Postponed » Active
Wim Leers’s picture

Version: 8.2.x-dev » 8.3.x-dev
Status: Active » Reviewed & tested by the community
FileSize
22.79 KB
Wim Leers’s picture

FileSize
23.31 KB

Oops, this is the right patch.

The last submitted patch, 8: 2835758-8.patch, failed testing.

Wim Leers’s picture

Issue summary: View changes
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed eea7ce5 and pushed to 8.3.x. Thanks!

  • alexpott committed eea7ce5 on 8.3.x
    Issue #2835758 by Wim Leers: Remove BigPipeInterface and move all of its...
Wim Leers’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.