I didn't realize it until now, but apparently Drupal now properly defines what constitutes an API and what doesn't. Apparently we should use @api and @internal to specify this in contrib modules, unless we want to use the default definitions (but even then being explicit will do no harm, I guess).
For everything considered part of the public API, needless to say, backwards-compatibility should as much as possible be maintained.
So, I guess, we should discuss what we want as part of the API and what not, and then walk through the code and tag accordingly (and/or add proper documentation for this). (I actually have already used @internal twice, without knowing Drupal even used that. So a small start is made, for DatabaseCompatibilityHandlerInterface and FieldInterface::setFieldIdentifier(), which really shouldn't be used by contrib modules.)
Some thoughts:
- Plugin implementations, I think, should in general never be considered an API. However, in the case of the Views integration this becomes a bit blurry, as other modules might very well want to build on the Search API-provided plugins. They shouldn't be required to implement all that logic on their own (though our filter/argument handlers are now a lot smaller than in D7, fortunately). Can they maybe at least rely on the traits? Or on what else?
- Do we want to make all interfaces part of the API (the two current exceptions mentioned above), or which don't make sense? Do we really guarantee the interfaces for services stay the same? Or might we add more methods and only support overriding by extending the default class? (Is that even "allowed" to say?)
- What else is there were we want to stray from the norm, or be extra-clear?
- What I planned in any case: beginning with the stable release, we should definitely start using change records to notify people when we even potentially break backwards-compatibility. But, of course, just having a CR doesn't mean we can just change whatever.
Quick side reading: Wim Leers' presentation about this
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2871549-2--what_is_api.patch | 605 bytes | drunken monkey |
Comments
Comment #2
drunken monkeyMaybe for now, we could just add a note to the
README.txt? (And, maybe to our developer documentation.)Comment #3
borisson_I agree with making sure that all plugins are @internal, not sure about other interfaces / classes yet. I'll have a look at that on saturday.
Comment #4
borisson_Actually, I just read the #2562903: Drupal core backend backwards compatibility and internal API policy and I think that properly discusses the things that makes sense (plugins, public properties and constructors are internal)
Comment #5
drunken monkeyThanks for your feedback!
OK, then let's get this in right away, and then we can discuss later (I'll keep it in my TODOs) what changes we might want to make.
Comment #7
drunken monkeyComment #8
drunken monkeyI just got an email from someone who's porting the VBO module, wants to include Search API support and (amongst other things) wanted to know whether our
ResultRowstructure is stable. Which is a very good question, actually, and something we might want to clearly define?My first thought was "Yes, of course.", but then I realized I'm actually proposing a change right now, in #2650986: Fix entity loading in SearchApiFieldTrait. It's more of an oversight, really, and now it would look to me like the structure is stable, but it's still not really certain. And we definitely neither properly describe it anywhere, nor define how stable it can be considered to be.