Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

Status: Active » Needs review
FileSize
50.78 KB

Experimental patch... Wonna see what happens with the bot :)

Status: Needs review » Needs work

The last submitted patch, 1537434-user-entity-type-hinting-1.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
50.47 KB

Reroll, will probably fail again. Have no idea why...

Status: Needs review » Needs work

The last submitted patch, 1537434-user-entity-type-hinting-3.patch, failed testing.

aspilicious’s picture

Ok after installing I found the nastyness...
global $user is still a std object so most functions get both instances of User and std object. Resulting into failures...

aspilicious’s picture

Status: Needs work » Needs review
FileSize
46.87 KB

Ok this one should install...

Status: Needs review » Needs work

The last submitted patch, 1537434-user-entity-type-hinting-6.patch, failed testing.

tim.plunkett’s picture

Issue tags: +PSR-0

Tagging.

cosmicdreams’s picture

Status: Needs work » Needs review
Issue tags: -PSR-0

Status: Needs review » Needs work
Issue tags: +PSR-0

The last submitted patch, 1537434-user-entity-type-hinting-6.patch, failed testing.

cosmicdreams’s picture

Issue tags: +Needs reroll

needs reroll

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
46.54 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting-1537434-12.patch, failed testing.

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
390 bytes
46.59 KB

Forgot 'use' statement in user.module.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting-1537434-14.patch, failed testing.

Albert Volkman’s picture

Status: Needs work » Postponed

Postponing.

cosmicdreams’s picture

Status: Postponed » Needs review
FileSize
33.69 KB

Rerolled, provided type hinting in user.module. Checking to see how much this breaks.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting.patch, failed testing.

cosmicdreams’s picture

Status: Needs work » Needs review
FileSize
56.29 KB

After reviewing the patch I found many more places where type hinting could be used.

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting_19.patch, failed testing.

aspilicious’s picture

Status: Needs work » Postponed

This was postponed because session handling isn't a true entity. This will fail all the time. Leave it postponed untill we figured out the session handling.

tim.plunkett’s picture

Status: Postponed » Needs work
socketwench’s picture

Status: Needs work » Needs review
FileSize
34.21 KB

Rerolled. I hope.

I wonder if I can beg someone to show me how to do this in PHPstorm. Doing manually is just too error prone.

socketwench’s picture

Might need to redo the patch because of https://drupal.org/node/2053489

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting_24.patch, failed testing.

garphy’s picture

Issue tags: -Needs reroll
FileSize
34.23 KB

Rerolled

garphy’s picture

Status: Needs work » Needs review

changing status to trigger the bot

Status: Needs review » Needs work

The last submitted patch, user_entity_type_hinting_27.patch, failed testing.

cosmicdreams’s picture

+++ b/core/includes/bootstrap.inc
@@ -2848,7 +2848,7 @@ function drupal_classloader_register($name, $path) {
+ * function user_access($string, User $account = NULL) {

In general, why are we using User and not UserInterface to type these parameters?

So, I think we need to document the parameters in the function's docblock.

cosmicdreams’s picture

Status: Needs work » Needs review
FileSize
36.12 KB

Here's a patch that explains what I'm saying.

Status: Needs review » Needs work

The last submitted patch, 1537434_31.patch, failed testing.

Berdir’s picture

  1. +++ b/core/modules/comment/comment.module
    @@ -1211,8 +1212,65 @@ function comment_user_cancel($edit, $account, $method) {
    -function comment_user_predelete($account) {
    +function comment_user_predelete(User $account) {
    

    Yes, UserInterface should be used, for example here too :)

    Lots of those below, always UserInterface, in @params and hook_*() documentations always fully qualified with a leading \, in actual implementations only UserInterface + a use.

  2. +++ b/core/modules/comment/comment.module
    @@ -1211,8 +1212,65 @@ function comment_user_cancel($edit, $account, $method) {
    +
    +/**
    + * Accepts a submission of new or changed comment content.
    + *
    + * @param Drupal\comment\Comment $comment
    + *   A comment object.
    + */
    +function comment_save(Comment $comment) {
    +  $comment->save();
    +}
    

    I have absolutely no clue where all this is coming from (not just this, all the added functions around this), but it shouldn't be here :)

cosmicdreams’s picture

Probably just how I created the patch. Clearly, there's more work to be done.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dpi’s picture

Issue summary: View changes
Status: Needs work » Closed (outdated)

Safe to say this is outdated.