Sub-Task of #1862398: [meta] Replace drupal_http_request() with Guzzle.

CommentFileSizeAuthor
#10 statistics-guzzle-1867382-10.patch8.29 KBBerdir
#9 statistics-guzzle-1867382-9.patch8.38 KBSutharsan
#1 statistics-guzzle-1867382-1.patch8.76 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
FileSize
statistics-guzzle-1867382-1.patch8.76 KB

Another simple conversion. Not sure why we specify the timeout explicitly, but I kept it for the moment. Might also be useful to ave a helper method for those calls but that would require a common base class.

Put the client config in setUp() if there was more than one call and directly in the code for those that only do a single call.

rbayliss’s picture

rbayliss CreditAttribution: rbayliss commented

Looks good to me.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood commented

#1: statistics-guzzle-1867382-1.patch queued for re-testing.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood commented
Status: Needs review » Reviewed & tested by the community

Looks good.

Dries’s picture

Dries CreditAttribution: Dries commented

Asking for a retest. Patch doesn't seem to apply.

Dries’s picture

Dries CreditAttribution: Dries commented

#1: statistics-guzzle-1867382-1.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, statistics-guzzle-1867382-1.patch, failed testing.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue tags: +Novice

Might work as a novice issue to reroll this (helpful doc on how to do rerolls http://drupal.org/patch/reroll)

Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan commented
Status: Needs work » Needs review
Issue tags: -Novice
FileSize
statistics-guzzle-1867382-9.patch8.38 KB

Patch rerolled.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
FileSize
statistics-guzzle-1867382-10.patch8.29 KB

Another re-roll, to use Drupal::httpClient()

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Status: Needs review » Reviewed & tested by the community

standards-wise this looks good.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed e45b1a7 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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