Follow-up to #2873395: [meta] Add @internal to core classes, methods, properties

Problem/Motivation

https://www.drupal.org/node/2562903#schema

Functions and class methods that define schema for core modules, should not be considered part of the public API. Most of schema definitions are now in YAML files; however, there is a bunch of methods called `schemaDefinition()`. All of them should have `@internal` tag

Proposed resolution

Add @internal tag to the class methods named schemaDefinition()

How to fix this issue

  1. Read the description for a category in its issue
  2. Identify and confirm an example. Ask in IRC if unclear.
  3. Search core for the relevant category.
  4. Add @internal per the backwards compatibility policy.
  5. Reviewers should confirm that each @internal mention is appropriate for that category according to the policy.

Remaining tasks

Make the Patch and post it

User interface changes

None.

API changes

There should be no implicit API changes.

CommentFileSizeAuthor
#5 2912636-5.patch9.45 KBwengerk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

valthebald created an issue. See original summary.

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald commented
Title: Add @internal to core classes, methods, properties to Plugins » Add @internal to core classes, methods, properties to schema defintion methods
valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald commented
Title: Add @internal to core classes, methods, properties to schema defintion methods » Add @internal to core classes and methods that return schema definition
wengerk’s picture

wengerk
he/him
Primary language French
Location Lausanne
CreditAttribution: wengerk at Antistatique commented

Working on this issue during the DrupalConEu Vienna.

wengerk’s picture

wengerk
he/him
Primary language French
Location Lausanne
CreditAttribution: wengerk at Antistatique commented
FileSize
2912636-5.patch9.45 KB

Made by @wengerk & review by @gido.

wengerk’s picture

wengerk
he/him
Primary language French
Location Lausanne
CreditAttribution: wengerk at Antistatique commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2912636-5.patch, failed testing. View results

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald commented
Status: Needs work » Needs review

CI bot hiccup, sending for retest

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald commented
Title: Add @internal to core classes and methods that return schema definition » Add @internal to schemaDefinition() methods
Status: Needs review » Reviewed & tested by the community

After applying the patch, I have searched the core for occurences of schemaDefinition functions, and all of them (excluding child classes that override parent's method) have @internal tag. Changing the issue title (narrowing the scope, in case someone will identify other functions that return schema definitions).

Moving to RTBC

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald commented
Issue summary: View changes
gido’s picture

gido CreditAttribution: gido as a volunteer commented

As a side note, those classes were not touched by purpose:

* Drupal\Tests\views\Kernel\Handler\FieldDateTest
* Drupal\Tests\views\Kernel\Handler\FilterBooleanOperatorStringTest
* Drupal\Tests\language\Kernel\Views\LanguageTestBase
* Drupal\Core\ProxyClass\Lock\PersistentDatabaseLockBackend

They all have the {@inheritdoc} doc annotation

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald commented

2 more classes that are ignored for the same reason:

* Drupal\Core\ProxyClass\Batch\BatchStorage
* Drupal\Core\ProxyClass\Lock\DatabaseLockBackend

  • lauriii committed df0b92d on 8.5.x 
    Issue #2912636 by wengerk, valthebald, gido: Add @internal to...

lauriii credited lauriii.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Committed as part of the live commit in DrupalCon Vienna df0b92d and pushed to 8.5.x and to 8.4.x with the permission of @webchick. Thanks!

  • lauriii committed 9569d55 on 8.4.x 
    Issue #2912636 by wengerk, valthebald, gido: Add @internal to...

Status: Fixed » Closed (fixed)

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