Mark procedural wrappers in entity.inc as deprecated

deprecating methods in entity.inc.

Marked the old #2135693: Deprecate wrappers in entity.inc as a duplicate.

A group of us are working on this (reviewing) at the Drupacon Sprint.
Update: May 15, 2015 at 12:10pm - Patch needs work, the actual status is unclear because there is a mismatch (discrepancy) in the code compared to the issue summary. There is a method (entity_load_multiple) which is commented as deprecated at line 151 in the patch that is not in the issue summary. Is this just scope creep or was that method inadvertently omitted from the issue summary?

Moving back to needs review. In regards to #8, I believe it was mistakenly left out of issue summary, given this meta issue: #2225347: [META] Replace calls to ENTITY_TYPE_load() and ENTITY_TYPE_load_multiple() with static method calls

Thanks folks!

In each case, we need to also document what should be used in place of the function.

Documented at all the places.Needs Reviewers Eyes.
Note : At some places the comments has gone beyond 80 chars.So thoughts ?

Needs a reroll.

$ git apply entity_wrapper_mark_procedural-2474151-14.patch 
error: patch failed: core/includes/entity.inc:213
error: core/includes/entity.inc: patch does not apply

Rerolled the patch as per D8 latest release.

I'm not sure whether deprecations are still happening, but these make sense, especially given the how-to info in the @deprecated annotation.

Just a couple things:

1. +++ b/core/includes/entity.inc
   @@ -60,6 +65,9 @@ function entity_get_bundles($entity_type = NULL) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   \Drupal::entityManager()->getStorage($entity_type) to load the entity.
   

   \Drupal::entityManager()->getStorage($type)->load();

2. +++ b/core/includes/entity.inc
   @@ -242,6 +270,10 @@ function entity_create($entity_type, array $values = array()) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   Entity Class label function by injecting it.
   + *   return $entity->label($langcode);
   

   Not sure what's meant here 'by injecting it.' It should just say "Use $entity->label($langcode);"

The changes suggested in #18

+++ b/core/includes/entity.inc
@@ -60,6 +65,9 @@ function entity_get_bundles($entity_type = NULL) {
+ *
+ * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
+ *   \Drupal::entityManager()->getStorage($type)->load(); ¶

There's whitespace at the end of the line.

So sorry, that was careless. Thanks of the guidance.

1. +++ b/core/includes/entity.inc
   @@ -60,6 +65,9 @@ function entity_get_bundles($entity_type = NULL) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   \Drupal::entityManager()->getStorage($type)->load();
     */
    function entity_load($entity_type, $id, $reset = FALSE) {
   
   @@ -138,6 +152,9 @@ function entity_revision_delete($entity_type, $revision_id) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   \Drupal::entityManager()->getStorage($entity_type)->loadMultiple($ids);
     */
    function entity_load_multiple($entity_type, array $ids = NULL, $reset = FALSE) {
   

   See the current docs for entity_create(), these methods should reference

   ::load()

   and loadMultiple()

2. +++ b/core/includes/entity.inc
   @@ -214,10 +243,9 @@ function entity_delete_multiple($entity_type, array $ids) {
   - * @deprecated in Drupal 8.x-dev, will be removed before Drupal 9.0.0. Use
   - *    the <EntityType>::create($values) method if the entity type is known or
   - *    \Drupal::entityManager()->getStorage($entity_type)->create($values) if the
   - *    entity type is variable.
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   EntityManager Create Method to create Entity.
   + *   return \Drupal::entityManager()->getStorage($entity_type)->create($values);
   

   Please don't change this.

Thank you, Tim. I have made the changes you suggested in #22

 * @deprecated in Drupal 8.x-dev, will be removed before Drupal 9.0.0. Use
 *    the <EntityType>::create($values) method if the entity type is known or
 *    \Drupal::entityManager()->getStorage($entity_type)->create($values) if the
 *    entity type is variable.

So this should say 8.0.x instead of 8.x-dev... But the usage info should stay the same. :-)

Other than that, RTBC.

Made change and marked RTBC from #24

+++ b/core/includes/entity.inc
@@ -195,6 +222,12 @@ function entity_load_unchanged($entity_type, $id) {
+ *   EntityManager loadMultiple Method to delete multiple entities

@@ -263,6 +299,12 @@ function entity_page_label(EntityInterface $entity, $langcode = NULL) {
+ *   Entity Manager view method for it after building the entity view using
+ *   getViewBuilder method.

@@ -289,6 +331,12 @@ function entity_view(EntityInterface $entity, $view_mode, $langcode = NULL, $res
+ *   Entity Manager viewMultiple method for it after building the entity view
+ *   using getViewBuilder method.

@@ -336,6 +384,9 @@ function entity_view_multiple(array $entities, $view_mode, $langcode = NULL, $re
+ *   EntityManager load function to load entity specific view display.

@@ -392,6 +443,9 @@ function entity_get_display($entity_type, $bundle, $view_mode) {
+ *   EntityManager load function to load its specific form display.

The first half of this patch just says "Use CODE HERE" or has an actual sentence explaining the choices. These last few are not English sentences, and they can probably be removed, and should have the code example

Also, please include an interdiff, and don't RTBC your own patches. Thanks!

Patch created on the suggestion of #26. Please review.

Hi @sorabh.v6,

Thank you for your patch. The comments seem already improved, as suggested by @tim.plunkett in #26, but the patch also contains a lot of changes which are not necessary for this issue.

+++ b/core/includes/entity.inc
@@ -392,6 +452,18 @@ function entity_get_display($entity_type, $bundle, $view_mode) {
diff --git a/core/lib/Drupal/Core/Block/BlockBase.php b/core/lib/Drupal/Core/Block/BlockBase.php

diff --git a/core/lib/Drupal/Core/Block/BlockBase.php b/core/lib/Drupal/Core/Block/BlockBase.php
index 262fd0c..9644dd1 100644

index 262fd0c..9644dd1 100644
--- a/core/lib/Drupal/Core/Block/BlockBase.php

--- a/core/lib/Drupal/Core/Block/BlockBase.php
+++ b/core/lib/Drupal/Core/Block/BlockBase.php

+++ b/core/lib/Drupal/Core/Block/BlockBase.php
+++ b/core/lib/Drupal/Core/Block/BlockBase.php
@@ -162,24 +162,29 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta

@@ -162,24 +162,29 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
       '#value' => $definition['provider'],
     );
 
-    $form['admin_label'] = array(
-      '#type' => 'item',
-      '#title' => $this->t('Block description'),
-      '#markup' => SafeMarkup::checkPlain($definition['admin_label']),
-    );
+    // The following settings should appear before plugin-specific settings
     $form['label'] = array(
       '#type' => 'textfield',
       '#title' => $this->t('Title'),
       '#maxlength' => 255,
       '#default_value' => $this->label(),
       '#required' => TRUE,
+      '#weight' => -4,
     );
     $form['label_display'] = array(
       '#type' => 'checkbox',
       '#title' => $this->t('Display title'),
       '#default_value' => ($this->configuration['label_display'] === BlockInterface::BLOCK_LABEL_VISIBLE),
       '#return_value' => BlockInterface::BLOCK_LABEL_VISIBLE,
+      '#weight' => -3,
+    );
+    $form['admin_label'] = array(
+      '#type' => 'item',
+      '#title' => $this->t('Block description'),
+      '#markup' => SafeMarkup::checkPlain($definition['admin_label']),
+      '#weight' => -2,
     );
+    // The following settings should appear after plugin-specific settings
     // Identical options to the ones for page caching.
     // @see \Drupal\system\Form\PerformanceForm::buildForm()
     $period = array(0, 60, 180, 300, 600, 900, 1800, 2700, 3600, 10800, 21600, 32400, 43200, 86400);
@@ -189,6 +194,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta

@@ -189,6 +194,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
     $form['cache'] = array(
       '#type' => 'details',
       '#title' => $this->t('Cache settings'),
+      '#weight' => 3,
     );
     $form['cache']['max_age'] = array(
       '#type' => 'select',
diff --git a/core/modules/block/src/BlockForm.php b/core/modules/block/src/BlockForm.php

diff --git a/core/modules/block/src/BlockForm.php b/core/modules/block/src/BlockForm.php
index ac9406a..efc8817 100644

index ac9406a..efc8817 100644
--- a/core/modules/block/src/BlockForm.php

--- a/core/modules/block/src/BlockForm.php
+++ b/core/modules/block/src/BlockForm.php

+++ b/core/modules/block/src/BlockForm.php
+++ b/core/modules/block/src/BlockForm.php
@@ -120,8 +120,20 @@ public function form(array $form, FormStateInterface $form_state) {

@@ -120,8 +120,20 @@ public function form(array $form, FormStateInterface $form_state) {
     $form_state->setTemporaryValue('gathered_contexts', $this->dispatcher->dispatch(BlockEvents::ADMINISTRATIVE_CONTEXT, new BlockContextEvent())->getContexts());
 
     $form['#tree'] = TRUE;
-    $form['settings'] = $entity->getPlugin()->buildConfigurationForm(array(), $form_state);
+    $form = array_merge($entity->getPlugin()->buildConfigurationForm(array(), $form_state), $form);
+    $form['region'] = array(
+      '#type' => 'select',
+      '#title' => $this->t('Region'),
+      '#description' => $this->t('Select the region where this block should be displayed.'),
+      '#default_value' => $entity->getRegion(),
+      '#empty_value' => BlockInterface::BLOCK_REGION_NONE,
+      '#options' => system_region_list($theme, REGIONS_VISIBLE),
+      '#prefix' => '<div id="edit-block-region-wrapper">',
+      '#suffix' => '</div>',
+      '#weight' => 0,
+    );
     $form['visibility'] = $this->buildVisibilityInterface([], $form_state);
+    $form['visibility']['#weight'] = 2;
 
     // If creating a new block, calculate a safe default machine name.
     $form['id'] = array(
@@ -164,17 +176,6 @@ public function form(array $form, FormStateInterface $form_state) {

@@ -164,17 +176,6 @@ public function form(array $form, FormStateInterface $form_state) {
       );
     }
 
-    // Region settings.
-    $form['region'] = array(
-      '#type' => 'select',
-      '#title' => $this->t('Region'),
-      '#description' => $this->t('Select the region where this block should be displayed.'),
-      '#default_value' => $entity->getRegion(),
-      '#empty_value' => BlockInterface::BLOCK_REGION_NONE,
-      '#options' => system_region_list($theme, REGIONS_VISIBLE),
-      '#prefix' => '<div id="edit-block-region-wrapper">',
-      '#suffix' => '</div>',
-    );
     $form['#attached']['library'][] = 'block/drupal.block.admin';
     return $form;
   }
@@ -275,14 +276,8 @@ protected function actions(array $form, FormStateInterface $form_state) {

@@ -275,14 +276,8 @@ protected function actions(array $form, FormStateInterface $form_state) {
    */
   public function validate(array $form, FormStateInterface $form_state) {
     parent::validate($form, $form_state);
-
-    // The Block Entity form puts all block plugin form elements in the
-    // settings form element, so just pass that to the block for validation.
-    $settings = (new FormState())->setValues($form_state->getValue('settings'));
     // Call the plugin validate handler.
-    $this->entity->getPlugin()->validateConfigurationForm($form, $settings);
-    // Update the original form values.
-    $form_state->setValue('settings', $settings->getValues());
+    $this->entity->getPlugin()->validateConfigurationForm($form, $form_state);
     $this->validateVisibility($form, $form_state);
   }
 
@@ -321,15 +316,9 @@ public function submitForm(array &$form, FormStateInterface $form_state) {

@@ -321,15 +316,9 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
     parent::submitForm($form, $form_state);
 
     $entity = $this->entity;
-    // The Block Entity form puts all block plugin form elements in the
-    // settings form element, so just pass that to the block for submission.
-    // @todo Find a way to avoid this manipulation.
-    $settings = (new FormState())->setValues($form_state->getValue('settings'));
 
     // Call the plugin submit handler.
-    $entity->getPlugin()->submitConfigurationForm($form, $settings);
-    // Update the original form values.
-    $form_state->setValue('settings', $settings->getValues());
+    $entity->getPlugin()->submitConfigurationForm($form, $form_state);
 
     // Submit visibility condition settings.
     foreach ($form_state->getValue('visibility') as $condition_id => $values) {
diff --git a/core/modules/block/src/Tests/BlockInterfaceTest.php b/core/modules/block/src/Tests/BlockInterfaceTest.php

diff --git a/core/modules/block/src/Tests/BlockInterfaceTest.php b/core/modules/block/src/Tests/BlockInterfaceTest.php
index 5e8b54c..04d9fed 100644

index 5e8b54c..04d9fed 100644
--- a/core/modules/block/src/Tests/BlockInterfaceTest.php

--- a/core/modules/block/src/Tests/BlockInterfaceTest.php
+++ b/core/modules/block/src/Tests/BlockInterfaceTest.php

+++ b/core/modules/block/src/Tests/BlockInterfaceTest.php
+++ b/core/modules/block/src/Tests/BlockInterfaceTest.php
@@ -65,32 +65,37 @@ public function testBlockInterface() {

@@ -65,32 +65,37 @@ public function testBlockInterface() {
     $period = array_map(array(\Drupal::service('date.formatter'), 'formatInterval'), array_combine($period, $period));
     $period[0] = '<' . t('no caching') . '>';
     $period[\Drupal\Core\Cache\Cache::PERMANENT] = t('Forever');
+    $weight = -4;
     $expected_form = array(
       'provider' => array(
         '#type' => 'value',
         '#value' => 'block_test',
       ),
-      'admin_label' => array(
-        '#type' => 'item',
-        '#title' => t('Block description'),
-        '#markup' => SafeMarkup::checkPlain($definition['admin_label']),
-      ),
       'label' => array(
         '#type' => 'textfield',
         '#title' => 'Title',
         '#maxlength' => 255,
         '#default_value' => 'Custom Display Message',
         '#required' => TRUE,
+        '#weight' => $weight++,
       ),
       'label_display' => array(
         '#type' => 'checkbox',
         '#title' => 'Display title',
         '#default_value' => TRUE,
         '#return_value' => 'visible',
+        '#weight' => $weight++,
+      ),
+      'admin_label' => array(
+        '#type' => 'item',
+        '#title' => t('Block description'),
+        '#markup' => SafeMarkup::checkPlain($definition['admin_label']),
+        '#weight' => $weight++,
       ),
       'cache' => array(
         '#type' => 'details',
         '#title' => t('Cache settings'),
+        '#weight' => 3,
         'max_age' => array(
           '#type' => 'select',
           '#title' => t('Maximum age'),
diff --git a/core/modules/block/src/Tests/BlockTest.php b/core/modules/block/src/Tests/BlockTest.php

diff --git a/core/modules/block/src/Tests/BlockTest.php b/core/modules/block/src/Tests/BlockTest.php
index d6e0999..559df5d 100644

index d6e0999..559df5d 100644
--- a/core/modules/block/src/Tests/BlockTest.php

--- a/core/modules/block/src/Tests/BlockTest.php
+++ b/core/modules/block/src/Tests/BlockTest.php

+++ b/core/modules/block/src/Tests/BlockTest.php
+++ b/core/modules/block/src/Tests/BlockTest.php
@@ -31,7 +31,7 @@ function testBlockVisibility() {

@@ -31,7 +31,7 @@ function testBlockVisibility() {
     $edit = array(
       'id' => strtolower($this->randomMachineName(8)),
       'region' => 'sidebar_first',
-      'settings[label]' => $title,
+      'label' => $title,
     );
     // Set the block to be hidden on any user path, and to be shown only to
     // authenticated users.
@@ -75,7 +75,7 @@ public function testBlockToggleVisibility() {

@@ -75,7 +75,7 @@ public function testBlockToggleVisibility() {
     $edit = array(
       'id' => strtolower($this->randomMachineName(8)),
       'region' => 'sidebar_first',
-      'settings[label]' => $title,
+      'label' => $title,
     );
     $block_id = $edit['id'];
     // Set the block to be shown only to authenticated users.
@@ -111,7 +111,7 @@ function testBlockVisibilityListedEmpty() {

@@ -111,7 +111,7 @@ function testBlockVisibilityListedEmpty() {
     $edit = array(
       'id' => strtolower($this->randomMachineName(8)),
       'region' => 'sidebar_first',
-      'settings[label]' => $title,
+      'label' => $title,
       'visibility[request_path][negate]' => TRUE,
     );
     // Set the block to be hidden on any user path, and to be shown only to
@@ -138,17 +138,17 @@ function testBlock() {

@@ -138,17 +138,17 @@ function testBlock() {
     // Select the 'Powered by Drupal' block to be configured and moved.
     $block = array();
     $block['id'] = 'system_powered_by_block';
-    $block['settings[label]'] = $this->randomMachineName(8);
+    $block['label'] = $this->randomMachineName(8);
     $block['theme'] = $this->config('system.theme')->get('default');
     $block['region'] = 'header';
 
     // Set block title to confirm that interface works and override any custom titles.
-    $this->drupalPostForm('admin/structure/block/add/' . $block['id'] . '/' . $block['theme'], array('settings[label]' => $block['settings[label]'], 'id' => $block['id'], 'region' => $block['region']), t('Save block'));
+    $this->drupalPostForm('admin/structure/block/add/' . $block['id'] . '/' . $block['theme'], array('label' => $block['label'], 'id' => $block['id'], 'region' => $block['region']), t('Save block'));
     $this->assertText(t('The block configuration has been saved.'), 'Block title set.');
     // Check to see if the block was created by checking its configuration.
     $instance = Block::load($block['id']);
 
-    $this->assertEqual($instance->label(), $block['settings[label]'], 'Stored block title found.');
+    $this->assertEqual($instance->label(), $block['label'], 'Stored block title found.');
 
     // Check whether the block can be moved to all available regions.
     foreach ($this->regions as $region) {
@@ -165,7 +165,7 @@ function testBlock() {

@@ -165,7 +165,7 @@ function testBlock() {
 
     // Confirm that the block instance title and markup are not displayed.
     $this->drupalGet('node');
-    $this->assertNoText(t($block['settings[label]']));
+    $this->assertNoText(t($block['label']));
     // Check for <div id="block-my-block-instance-name"> if the machine name
     // is my_block_instance_name.
     $xpath = $this->buildXPathQuery('//div[@id=:id]/*', array(':id' => 'block-' . str_replace('_', '-', strtolower($block['id']))));
@@ -174,9 +174,9 @@ function testBlock() {

@@ -174,9 +174,9 @@ function testBlock() {
     // Test deleting the block from the edit form.
     $this->drupalGet('admin/structure/block/manage/' . $block['id']);
     $this->clickLink(t('Delete'));
-    $this->assertRaw(t('Are you sure you want to delete the block %name?', array('%name' => $block['settings[label]'])));
+    $this->assertRaw(t('Are you sure you want to delete the block %name?', array('%name' => $block['label'])));
     $this->drupalPostForm(NULL, array(), t('Delete'));
-    $this->assertRaw(t('The block %name has been deleted.', array('%name' => $block['settings[label]'])));
+    $this->assertRaw(t('The block %name has been deleted.', array('%name' => $block['label'])));
 
     // Test deleting a block via "Configure block" link.
     $block = $this->drupalPlaceBlock('system_powered_by_block');
@@ -245,7 +245,7 @@ function testHideBlockTitle() {

@@ -245,7 +245,7 @@ function testHideBlockTitle() {
     $edit = array(
       'id' => $id,
       'region' => 'sidebar_first',
-      'settings[label]' => $title,
+      'label' => $title,
     );
     $this->drupalPostForm('admin/structure/block/add/' . $block_name . '/' . $default_theme, $edit, t('Save block'));
     $this->assertText('The block configuration has been saved.', 'Block was saved');
@@ -254,13 +254,13 @@ function testHideBlockTitle() {

@@ -254,13 +254,13 @@ function testHideBlockTitle() {
     $this->assertText($title, 'Block title was displayed by default.');
 
     $edit = array(
-      'settings[label_display]' => FALSE,
+      'label_display' => FALSE,
     );
     $this->drupalPostForm('admin/structure/block/manage/' . $id, $edit, t('Save block'));
     $this->assertText('The block configuration has been saved.', 'Block was saved');
 
     $this->drupalGet('admin/structure/block/manage/' . $id);
-    $this->assertNoFieldChecked('edit-settings-label-display', 'The display_block option has the correct default value on the configuration form.');
+    $this->assertNoFieldChecked('edit-label-display', 'The display_block option has the correct default value on the configuration form.');
 
     $this->drupalGet('user');
     $this->assertNoText($title, 'Block title was not displayed when hidden.');
@@ -290,7 +290,7 @@ function moveBlockToRegion(array $block, $region) {

@@ -290,7 +290,7 @@ function moveBlockToRegion(array $block, $region) {
 
     // Confirm that the block is being displayed.
     $this->drupalGet('');
-    $this->assertText(t($block['settings[label]']), 'Block successfully being displayed on the page.');
+    $this->assertText(t($block['label']), 'Block successfully being displayed on the page.');
 
     // Confirm that the custom block was found at the proper region.
     $xpath = $this->buildXPathQuery('//div[@class=:region-class]//div[@id=:block-id]/*', array(
diff --git a/core/modules/block/src/Tests/Views/DisplayBlockTest.php b/core/modules/block/src/Tests/Views/DisplayBlockTest.php

diff --git a/core/modules/block/src/Tests/Views/DisplayBlockTest.php b/core/modules/block/src/Tests/Views/DisplayBlockTest.php
index 3f96ab1..2d2b7c1 100644

index 3f96ab1..2d2b7c1 100644
--- a/core/modules/block/src/Tests/Views/DisplayBlockTest.php

--- a/core/modules/block/src/Tests/Views/DisplayBlockTest.php
+++ b/core/modules/block/src/Tests/Views/DisplayBlockTest.php

+++ b/core/modules/block/src/Tests/Views/DisplayBlockTest.php
+++ b/core/modules/block/src/Tests/Views/DisplayBlockTest.php
@@ -211,7 +211,7 @@ public function testViewsBlockForm() {

@@ -211,7 +211,7 @@ public function testViewsBlockForm() {
     // Tests the override capability of items per page.
     $this->drupalGet('admin/structure/block/add/views_block:test_view_block-block_1/' . $default_theme);
     $edit = array();
-    $edit['settings[override][items_per_page]'] = 10;
+    $edit['override[items_per_page]'] = 10;
 
     $this->drupalPostForm('admin/structure/block/add/views_block:test_view_block-block_1/' . $default_theme, $edit, t('Save block'));
 
@@ -219,7 +219,7 @@ public function testViewsBlockForm() {

@@ -219,7 +219,7 @@ public function testViewsBlockForm() {
     $config = $block->getPlugin()->getConfiguration();
     $this->assertEqual(10, $config['items_per_page'], "'Items per page' is properly saved.");
 
-    $edit['settings[override][items_per_page]'] = 5;
+    $edit['override[items_per_page]'] = 5;
     $this->drupalPostForm('admin/structure/block/manage/views_block__test_view_block_block_1_4', $edit, t('Save block'));
 
     $block = $storage->load('views_block__test_view_block_block_1_4');
@@ -229,8 +229,8 @@ public function testViewsBlockForm() {

@@ -229,8 +229,8 @@ public function testViewsBlockForm() {
 
     // Tests the override of the label capability.
     $edit = array();
-    $edit['settings[views_label_checkbox]'] = 1;
-    $edit['settings[views_label]'] = 'Custom title';
+    $edit['views_label_checkbox'] = 1;
+    $edit['views_label'] = 'Custom title';
     $this->drupalPostForm('admin/structure/block/add/views_block:test_view_block-block_1/' . $default_theme, $edit, t('Save block'));
 
     $block = $storage->load('views_block__test_view_block_block_1_5');
diff --git a/core/modules/block_content/src/Tests/BlockContentCreationTest.php b/core/modules/block_content/src/Tests/BlockContentCreationTest.php

diff --git a/core/modules/block_content/src/Tests/BlockContentCreationTest.php b/core/modules/block_content/src/Tests/BlockContentCreationTest.php
index 6bf250c..9363b6d 100644

index 6bf250c..9363b6d 100644
--- a/core/modules/block_content/src/Tests/BlockContentCreationTest.php

--- a/core/modules/block_content/src/Tests/BlockContentCreationTest.php
+++ b/core/modules/block_content/src/Tests/BlockContentCreationTest.php

+++ b/core/modules/block_content/src/Tests/BlockContentCreationTest.php
+++ b/core/modules/block_content/src/Tests/BlockContentCreationTest.php
@@ -54,7 +54,7 @@ public function testBlockContentCreation() {

@@ -54,7 +54,7 @@ public function testBlockContentCreation() {
     )), 'Basic block created.');
 
     // Check that the view mode setting is hidden because only one exists.
-    $this->assertNoFieldByXPath('//select[@name="settings[view_mode]"]', NULL, 'View mode setting hidden because only one exists');
+    $this->assertNoFieldByXPath('//select[@name="view_mode"]', NULL, 'View mode setting hidden because only one exists');
 
     // Check that the block exists in the database.
     $blocks = entity_load_multiple_by_properties('block_content', array('info' => $edit['info[0][value]']));
@@ -101,18 +101,18 @@ public function testBlockContentCreationMultipleViewModes() {

@@ -101,18 +101,18 @@ public function testBlockContentCreationMultipleViewModes() {
     )), 'Basic block created.');
 
     // Check that the view mode setting is shown because more than one exists.
-    $this->assertFieldByXPath('//select[@name="settings[view_mode]"]', NULL, 'View mode setting shown because multiple exist');
+    $this->assertFieldByXPath('//select[@name="view_mode"]', NULL, 'View mode setting shown because multiple exist');
 
     // Change the view mode.
-    $view_mode['settings[view_mode]'] = 'test_view_mode';
+    $view_mode['view_mode'] = 'test_view_mode';
     $this->drupalPostForm(NULL, $view_mode, t('Save block'));
 
     // Go to the configure page and verify that the new view mode is correct.
     $this->drupalGet('admin/structure/block/manage/testblock');
-    $this->assertFieldByXPath('//select[@name="settings[view_mode]"]/option[@selected="selected"]/@value', 'test_view_mode', 'View mode changed to Test View Mode');
+    $this->assertFieldByXPath('//select[@name="view_mode"]/option[@selected="selected"]/@value', 'test_view_mode', 'View mode changed to Test View Mode');
 
     // Test the available view mode options.
-    $this->assertOption('edit-settings-view-mode', 'default', 'The default view mode is available.');
+    $this->assertOption('edit-view-mode', 'default', 'The default view mode is available.');
 
     // Check that the block exists in the database.
     $blocks = entity_load_multiple_by_properties('block_content', array('info' => $edit['info[0][value]']));
@@ -206,7 +206,7 @@ public function testBlockDelete() {

@@ -206,7 +206,7 @@ public function testBlockDelete() {
     // Place the block.
     $instance = array(
       'id' => Unicode::strtolower($edit['info[0][value]']),
-      'settings[label]' => $edit['info[0][value]'],
+      'label' => $edit['info[0][value]'],
       'region' => 'sidebar_first',
     );
     $block = BlockContent::load(1);
@@ -260,7 +260,7 @@ public function testConfigDependencies() {

@@ -260,7 +260,7 @@ public function testConfigDependencies() {
     $block_placement_id = Unicode::strtolower($block->label());
     $instance = array(
       'id' => $block_placement_id,
-      'settings[label]' => $block->label(),
+      'label' => $block->label(),
       'region' => 'sidebar_first',
     );
     $block = BlockContent::load(1);
diff --git a/core/modules/menu_ui/src/Tests/MenuTest.php b/core/modules/menu_ui/src/Tests/MenuTest.php

diff --git a/core/modules/menu_ui/src/Tests/MenuTest.php b/core/modules/menu_ui/src/Tests/MenuTest.php
index 59616e7..949f33a 100644

index 59616e7..949f33a 100644
--- a/core/modules/menu_ui/src/Tests/MenuTest.php

--- a/core/modules/menu_ui/src/Tests/MenuTest.php
+++ b/core/modules/menu_ui/src/Tests/MenuTest.php

+++ b/core/modules/menu_ui/src/Tests/MenuTest.php
+++ b/core/modules/menu_ui/src/Tests/MenuTest.php
@@ -947,8 +947,8 @@ protected function doTestMenuBlock() {

@@ -947,8 +947,8 @@ protected function doTestMenuBlock() {
     $block_id = $this->blockPlacements[$menu_id];
     $this->drupalGet('admin/structure/block/manage/' . $block_id);
     $this->drupalPostForm(NULL, [
-      'settings[depth]' => 3,
-      'settings[level]' => 2,
+      'depth' => 3,
+      'level' => 2,
     ], t('Save block'));
     $block = Block::load($block_id);
     $settings = $block->getPlugin()->getConfiguration();
diff --git a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php

diff --git a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php
index aec9716..d8a0f9c 100644

index aec9716..d8a0f9c 100644
--- a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php

--- a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php
+++ b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php

+++ b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php
+++ b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php
@@ -86,6 +86,7 @@ public function blockForm($form, FormStateInterface $form_state) {

@@ -86,6 +86,7 @@ public function blockForm($form, FormStateInterface $form_state) {
       // Open if not set to defaults.
       '#open' => $defaults['level'] !== $config['level'] || $defaults['depth'] !== $config['depth'],
       '#process' => [[get_class(), 'processMenuLevelParents']],
+      '#weight' => 1,
     );
 
     $options = range(0, $this->menuTree->maxDepth());
 

Could you create a new patch that only documents the deprecations? Thanks!

@JeroenT Sure, I'll do it.

1. +++ b/core/includes/entity.inc
   @@ -195,6 +222,12 @@ function entity_load_unchanged($entity_type, $id) {
   + *   $controller = \Drupal::entityManager()->getStorage($entity_type);
   + *   $entities = $controller->loadMultiple($ids);
   + *   $controller->delete($entities);
   

   storage is a handler not a controller

2. +++ b/core/includes/entity.inc
   @@ -263,6 +299,12 @@ function entity_page_label(EntityInterface $entity, $langcode = NULL) {
   + *   $render_controller = \Drupal::entityManager()->getViewBuilder($entity->getEntityTypeId());
   + *   return $render_controller->view($entity, $view_mode, $langcode);
   
   @@ -289,6 +331,12 @@ function entity_view(EntityInterface $entity, $view_mode, $langcode = NULL, $res
   + *   $render_controller = \Drupal::entityManager()->getViewBuilder($entity->getEntityTypeId());
   + *   return $render_controller->viewMultiple($entities, $view_mode, $langcode);
   

   s/render_controller/view_builder
   it's not about a render!

Created a new patch that contains the changes from patch 28 and the fixes as suggested by andypost in #31.

Interdiff is created from patch 25.

Just a couple coding standards issues:

1. +++ b/core/includes/entity.inc
   @@ -214,10 +247,10 @@ function entity_delete_multiple($entity_type, array $ids) {
   + * @deprecated in Drupal 8.0.x, will be removed before Drupal 9.0.0.
   + *   Use the <EntityType>::create($values) method if the entity type is known
   + *   or \Drupal::entityManager()->getStorage($entity_type)->create($values) if the
   + *   entity type is variable.
   

   Wrap at 80.

2. +++ b/core/includes/entity.inc
   @@ -289,6 +331,12 @@ function entity_view(EntityInterface $entity, $view_mode, $langcode = NULL, $res
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0.
   + *   Use Entity Manager viewMultiple method for it after building the entity view
   + *   using getViewBuilder method.
   + *   $view_builder = \Drupal::entityManager()->getViewBuilder($entity->getEntityTypeId());
   + *   return $view_builder->viewMultiple($entities, $view_mode, $langcode);
   

   Wrap at 80.

Regarding #33.2
$view_builder = \Drupal::entityManager()->getViewBuilder($entity->getEntityTypeId());
This should be single line so can't wrap it at 80.

Wrap the text at few other places where I found. Interdiff attached.

+++ b/core/includes/entity.inc
@@ -143,6 +159,11 @@ function entity_revision_delete($entity_type, $revision_id) {
+ *    \Drupal::entityManager()->getStorage($entity_type)->loadMultiple($ids) if the

Wrap at 80

Looks good and RTBC-ish. Thanks. :-)

However... Is this an unrelated change?

+++ b/core/modules/color/color.module
@@ -284,7 +284,7 @@ function template_preprocess_color_scheme_form(&$variables) {
 
 /**
diff --git a/core/modules/color/src/Tests/ColorSafePreviewTest.php b/core/modules/color/src/Tests/ColorSafePreviewTest.php

diff --git a/core/modules/color/src/Tests/ColorSafePreviewTest.php b/core/modules/color/src/Tests/ColorSafePreviewTest.php
deleted file mode 100644

deleted file mode 100644
index 9e42f96..0000000

index 9e42f96..0000000
--- a/core/modules/color/src/Tests/ColorSafePreviewTest.php

--- a/core/modules/color/src/Tests/ColorSafePreviewTest.php
+++ /dev/null

+++ /dev/null
+++ /dev/null
@@ -1,63 +0,0 @@

@@ -1,63 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\color\Tests\ColorSafePreviewTest.
- */

Oops. Also forgot to mention: #2526462: Mark entity_get_bundles() as @deprecated for 9.x. happened already.

Rerolled the patch #34-#38

The #37 not automatically merged using git pull.So need reroll

The patch in #39 applies, but you end up with 2 @deprecated blocks for entity_get_bundles(). :-)

ReRolled.

I don't think the re-roll on #41 was required since the patch in #38 could apply.

Created a new patch that doesn't add 2 deprecated blocks for entity_get_bundles().

And the patch is here!

Created a patch that's more in line with the patch in 1 #2526462: Mark entity_get_bundles() as @deprecated for 9.x.

Good deal. Just one thing (in two places):

+++ b/core/includes/entity.inc
@@ -341,6 +384,18 @@ function entity_view_multiple(array $entities, $view_mode, $langcode = NULL, $re
+ *   If display is available in configuration then use :

@@ -397,6 +452,18 @@ function entity_get_display($entity_type, $bundle, $view_mode) {
+ *   If display is available in configuration then use :

Colon should come right after 'use,' without any spaces in between.

Fixed feedback in #48.

Woot! Seeing Drupal CI on d.o! :-)

Anyway. Fixes all the stuff, does the deprecation, passes the tests that matter, and I'm sure the other tests are unrelated because this is a docs-only patch.

@Mile23, I'm sorry, I was too excited so I checked all the tests :p.

Thanks everyone. This is looking much improved! Some specific improvements for the current patch:

1. +++ b/core/includes/entity.inc
   @@ -61,6 +61,11 @@ function entity_get_bundles($entity_type = NULL) {
   + *   the <EntityType>::load($id) method if the entity type is known or
   + *   \Drupal::entityManager()->getStorage($entity_type)->load($id) if the
   

   Here and elsewhere: We should provide @see to \Drupal\fully\namespaced\Entity::load() and ditto for the storage interface load method, both at the end of the docblock. Reference: https://www.drupal.org/node/1354#see

   That would also allow us to make this a little clearer by saying "The method overriding Entity::load() for the entity type, e.g. \Drupal\....\Node::load()". Maybe that is clearer; what do you think?

   We might also want to add @code around the line with the chained method call. Something like:

   Use the method overriding... etc... if the entity type is known. If the entity type is variable, use the entity manager service to load the entity from the entity storage:
   @code
   \Drupal::entityManager()->...
   @endocde

2. +++ b/core/includes/entity.inc
   @@ -103,6 +111,9 @@ function entity_revision_load($entity_type, $revision_id) {
   + *   \Drupal::entityManager()>getStorage($entity_type)>deleteRevision($revision_id);
   

   Oops, missing a - from the ->. :)

3. +++ b/core/includes/entity.inc
   @@ -200,6 +222,12 @@ function entity_load_unchanged($entity_type, $id) {
   + *   EntityManager loadMultiple Method to delete multiple entities
   

   It's not actually the entity manager load multiple method. It's the method for the storage. Maybe something like: "Use the entity storage's loadMultiple() and delete() methods to delete multiple entities."

   I think there should also be an @see to the \Drupal\fully\namespaced EntityStorageInterface::delete() method (and similarly for loadMultiple()).

4. +++ b/core/includes/entity.inc
   @@ -200,6 +222,12 @@ function entity_load_unchanged($entity_type, $id) {
   + *   $storage_handler = \Drupal::entityManager()->getStorage($entity_type);
   + *   $entities = $storage_handler->loadMultiple($ids);
   + *   $storage_handler->delete($entities);
   

   Here and elsewhere in the patch: When we have multiple lines in the examples like this, we should use @code/@encode around them so they are formatted properly. Reference: https://www.drupal.org/node/1354#code

5. +++ b/core/includes/entity.inc
   @@ -233,8 +261,8 @@ function entity_create($entity_type, array $values = array()) {
   - * This is a wrapper for Drupal\Core\Entity\EntityInterface::label(). This function
   - * should only be used as a callback, e.g. for menu title callbacks.
   + * This is a wrapper for Drupal\Core\Entity\EntityInterface::label(). This
   + * function should only be used as a callback, e.g. for menu title callbacks.
   

   I think we can actually remove this paragraph. "Menu title callbacks" aren't a thing anymore and title callbacks can be methods.

6. +++ b/core/includes/entity.inc
   @@ -341,6 +384,18 @@ function entity_view_multiple(array $entities, $view_mode, $langcode = NULL, $re
   + *   If display is available in configuration then use:
   

   This is terse and a bit vague. We should explain it a little more clearly and also include articles and so on (like the word "the") so it's a complete sentence.

7. +++ b/core/includes/entity.inc
   @@ -397,6 +452,18 @@ function entity_get_display($entity_type, $bundle, $view_mode) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   If display is available in configuration then use:
   

   Something weird going on with the wording here.

1-5: Done
6-7: I tried to improve the comments for entity_get_display and entity_get_form_display, but I'm not sure it's completely correct.

Uploaded my patch too fast. Forgot to update the entity_load_multiple_by_properties comment + fixed some indenting problems.

It looks like all the items in #53 are addressed.

Just a few more things...

1. +++ b/core/includes/entity.inc
   @@ -86,8 +96,15 @@ function entity_load($entity_type, $id, $reset = FALSE) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   the entity storage's loadRevision method to load a specific entity
   + *   revision:
   

   loadRevision() (add the parentheses).

2. +++ b/core/includes/entity.inc
   @@ -103,6 +120,16 @@ function entity_revision_load($entity_type, $revision_id) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   the entity storage's deleteRevision method to delete a specific entity
   + *   revision:
   

   deleteRevision()

   There are a bunch of these. :-)

3. +++ b/core/includes/entity.inc
   @@ -164,6 +201,16 @@ function entity_load_multiple($entity_type, array $ids = NULL, $reset = FALSE) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   the entity storage's loadByProperties method to load an entity by their
   + *   property values:
   

   loadByProperties()

4. +++ b/core/includes/entity.inc
   @@ -184,8 +231,17 @@ function entity_load_multiple_by_properties($entity_type, array $values) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   the entity storage's loadUnchanged method to load an unchanged entity:
   

   loadUnchanged()

5. +++ b/core/includes/entity.inc
   @@ -200,6 +256,18 @@ function entity_load_unchanged($entity_type, $id) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   the entity storage's delete method to delete multiple entities:
   

   delete()

6. +++ b/core/includes/entity.inc
   @@ -219,10 +287,17 @@ function entity_delete_multiple($entity_type, array $ids) {
   + * @deprecated in Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   The method overriding Entity::create() for the entity type, e.g.
   + *   \Drupal\node\Entity\Node::create() if the entity type is known. If the
   + *   entity type is variable, use the entity storage's create method to
   + *   construct a new entity:
   

   create()

7. +++ b/core/includes/entity.inc
   @@ -243,9 +315,15 @@ function entity_create($entity_type, array $values = array()) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0. Use
   + *   the entity's label method to get the label of the entity:
   

   label()

8. +++ b/core/includes/entity.inc
   @@ -268,6 +346,16 @@ function entity_page_label(EntityInterface $entity, $langcode = NULL) {
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0.
   + *   Use the entity view builder's view method for creating a render array:
   

   view()

9. +++ b/core/includes/entity.inc
   @@ -294,6 +382,17 @@ function entity_view(EntityInterface $entity, $view_mode, $langcode = NULL, $res
   + * @deprecated as of Drupal 8.0.x, will be removed before Drupal 9.0.0.
   + *   Use the entity view builder's viewMultiple method for creating a render
   + *   array for the provided entities:
   

   viewMultiple()

@Mile23, thanks for the reviews.
addressing #56

Cool, thanks. :-)

Committed b7fe150 and pushed to 8.0.x. Thanks!