CommentFileSizeAuthor
#5 replace_t-2792171-5.patch6.81 KBShreya Shetty
#2 replace_t-2792171-2.patch6.81 KBShreya Shetty
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Shreya Shetty created an issue. See original summary.

Shreya Shetty’s picture

Shreya Shetty CreditAttribution: Shreya Shetty as a volunteer and at Trigyn Technologies Ltd commented
Status: Active » Needs review
FileSize
replace_t-2792171-2.patch6.81 KB
nicrodgers’s picture

nicrodgers
Primary language English
Location Monmouthshire, UK
CreditAttribution: nicrodgers at Welsh Government commented
Status: Needs review » Needs work

Just a few minor coding standard nitpicks:

  1. +++ b/src/Form/SessionLimitForm.php
@@ -44,9 +44,9 @@ class SessionLimitForm extends FormBase {
+      $message = $current_session_id == $obj->sid ?$this->t('Your current session.') : '';

    Need a space after the ? 

  2. +++ b/src/Form/SessionLimitForm.php
@@ -78,11 +78,11 @@ class SessionLimitForm extends FormBase {
+      $session_limit->sessionDisconnect($sid,$this->t('Your session was deliberately ended from another session.'));

    Need a space after the ,

nicrodgers’s picture

nicrodgers
Primary language English
Location Monmouthshire, UK
CreditAttribution: nicrodgers at Welsh Government commented
Version: 8.x-1.0-beta1 » 8.x-1.x-dev
Shreya Shetty’s picture

Shreya Shetty CreditAttribution: Shreya Shetty as a volunteer and at Trigyn Technologies Ltd commented
FileSize
replace_t-2792171-5.patch6.81 KB

Done as per your suggestions @nicrodgers . Thank You for the review.

Shreya Shetty’s picture

Shreya Shetty CreditAttribution: Shreya Shetty as a volunteer and at Trigyn Technologies Ltd commented
Status: Needs work » Needs review
nicrodgers’s picture

nicrodgers
Primary language English
Location Monmouthshire, UK
CreditAttribution: nicrodgers at Welsh Government commented
Status: Needs review » Reviewed & tested by the community

Looks good - nice one!

  • Shreya Shetty authored 0a0b30e on 8.x-1.x 
    Issue #2792171 by Shreya Shetty: Replace t() with this->t()
darksnow’s picture

darksnow CreditAttribution: darksnow at Deeson for Deeson commented
Component: Documentation » Code
Status: Reviewed & tested by the community » Fixed

Applies cleaning and looks good to me too, thanks for the check nicrodgers and thanks a lot for the patch Shreya Shetty

Merged patch #5

Status: Fixed » Closed (fixed)

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