Files: 
CommentFileSizeAuthor
#20 forum-remove-global-user-from-forum-module-2061935-20.patch1.84 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 58,695 pass(es). View
#16 forum-remove-global-user-from-forum-module-2061935-16.patch1.84 KBm1r1k
PASSED: [[SimpleTest]]: [MySQL] 59,034 pass(es). View
#12 forum-remove-global-user-from-forum-module-2061935-12.patch1.84 KBm1r1k
PASSED: [[SimpleTest]]: [MySQL] 58,031 pass(es). View
#12 interdiff.txt951 bytesm1r1k
#9 forum-remove-global-user-from-forum-module-2061935-9.patch1.88 KBm1r1k
FAILED: [[SimpleTest]]: [MySQL] 58,537 pass(es), 39 fail(s), and 17 exception(s). View
#2 forum-remove-global-user-from-forum-module-2061935-2.patch1.66 KBm1r1k
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch forum-remove-global-user-from-forum-module-2061935-2.patch. Unable to apply patch. See the log in the details link for more information. View
#1 forum-user-deprecated-2061935.patch1.66 KBdanylevskyi
PASSED: [[SimpleTest]]: [MySQL] 58,025 pass(es). View

Comments

danylevskyi’s picture

Assigned: danylevskyi » Unassigned
Status: Active » Needs review
FileSize
1.66 KB
PASSED: [[SimpleTest]]: [MySQL] 58,025 pass(es). View
m1r1k’s picture

FileSize
1.66 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch forum-remove-global-user-from-forum-module-2061935-2.patch. Unable to apply patch. See the log in the details link for more information. View
m1r1k’s picture

Assigned: Unassigned » m1r1k

For easier trakcing

danylevskyi’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

patch no longer applies.

m1r1k’s picture

Status: Needs work » Needs review

Push to retesting

m1r1k’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll, +CodeSprintCIS

The last submitted patch, forum-remove-global-user-from-forum-module-2061935-2.patch, failed testing.

m1r1k’s picture

Status: Needs work » Needs review
FileSize
1.88 KB
FAILED: [[SimpleTest]]: [MySQL] 58,537 pass(es), 39 fail(s), and 17 exception(s). View

Rerolled

Status: Needs review » Needs work

The last submitted patch, forum-remove-global-user-from-forum-module-2061935-9.patch, failed testing.

Cottser’s picture

Thanks for working on this @m1r1k! This needs another reroll, the patch applies with the `patch` command but not with `git apply`. Testbot uses `git apply`.

  1. +++ b/core/modules/forum/lib/Drupal/forum/ForumManager.php
    @@ -139,7 +139,8 @@ public function getTopics($tid) {
    +    $user = $user = Drupal::currentUser();
    

    Just one $user = :)

  2. +++ b/core/modules/forum/lib/Drupal/forum/ForumManager.php
    @@ -326,7 +327,7 @@ protected function numberNew($nid, $timestamp) {
    +    $user = Drupal::currentUser();
    

    This one and the above in the ForumManager class should be \Drupal:: since they are within a namespaced class and we don't "use Drupal". That would likely explain the test failures. See https://drupal.org/node/1353118 for more information.

m1r1k’s picture

FileSize
951 bytes
1.84 KB
PASSED: [[SimpleTest]]: [MySQL] 58,031 pass(es). View

Oh, my fault. "Haste makes waste" ... @Cottser, thank you for corrections.
Fixed

m1r1k’s picture

Status: Needs work » Needs review
Cottser’s picture

Issue tags: -Needs reroll

I'm seeing a couple other conversions are using this in OO code, but I really don't know which is the correct/preferred method.

$user = $this->container->get('current_user');
m1r1k’s picture

@Cotteser, afaik such code is using inside some OO code that have its own container, like Tests or some plugins which injects container directly.

m1r1k’s picture

FileSize
1.84 KB
PASSED: [[SimpleTest]]: [MySQL] 59,034 pass(es). View

Re-roll after deciding use '\Drupal' everywhere

The last submitted patch, forum-remove-global-user-from-forum-module-2061935-16.patch, failed testing.

m1r1k’s picture

Status: Needs work » Needs review
Issue tags: +CodeSprintCIS
joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Applies, passes testbot and replaces all within the module. Ready to go, thanks @m1r1k!

joelpittet’s picture

Assigned: m1r1k » Unassigned
Status: Reviewed & tested by the community » Needs review
FileSize
1.84 KB
PASSED: [[SimpleTest]]: [MySQL] 58,695 pass(es). View

Well it doesn't apply any more :S so reroll attached. Someone else to review.

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -CodeSprintCIS

back to RTBC, patch still passes after re-roll.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ba69ad0 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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