CommentFileSizeAuthor
#5 2759401-5.patch1.38 KBnaveenvalecha
#2 2759401-2.patch4.69 KBrajeshwari10
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rajeshwari10 created an issue. See original summary.

rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
Assigned: rajeshwari10 » Unassigned
Status: Active » Needs review
FileSize
2759401-2.patch4.69 KB

Removed deprecated db_select.

vaibhavjain’s picture

vaibhavjain
he/him
Primary language Hindi
Location Delhi
CreditAttribution: vaibhavjain at Acquia commented
Status: Needs review » Needs work 
+++ b/S3fsStreamWrapper.inc
@@ -964,6 +964,7 @@ class S3fsStreamWrapper implements DrupalStreamWrapperInterface {
+    $connection = \Drupal\Core\Database\Database::getConnection();

Rather than instantiating the connection object here, can you instantiate an object in the constructor and use it further.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented
Assigned: Unassigned » naveenvalecha 
+++ b/tests/s3fs.test
@@ -150,6 +150,7 @@ else {
      * Test the cache refresh.

This file is gonna removed in #2758493: Remove legacy/old tests folder. So need to do its changes here.

Assingning to myself to address #3

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented
Assigned: naveenvalecha » Unassigned
Status: Needs work » Needs review
FileSize
2759401-5.patch1.38 KB 
+++ b/S3fsStreamWrapper.inc
+++ b/S3fsStreamWrapper.inc
@@ -964,6 +964,7 @@ class S3fsStreamWrapper implements DrupalStreamWrapperInterface {

The S3fsStreamWrapper class is of no use as DrupalStreamWrapperInterface does not exist in 8.x so this needs updation as per 8.x. So these changes seems not required here.

neetu morwani’s picture

neetu morwani CreditAttribution: neetu morwani as a volunteer and at Acquia commented

Patch looks good.
Thanks for the work Naveen.

neetu morwani’s picture

neetu morwani CreditAttribution: neetu morwani as a volunteer and at Acquia commented
Status: Needs review » Reviewed & tested by the community

The last submitted patch, 2: 2759401-2.patch, failed testing.

abhishek-anand’s picture

abhishek-anand CreditAttribution: abhishek-anand at Acquia commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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