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

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new 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 commented

Looks good to me.

timmillwood’s picture

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

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

timmillwood’s picture

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

Looks good.

dries’s picture

dries commented

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

dries’s picture

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 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 commented
Status: Needs work » Needs review
Issue tags: -Novice
StatusFileSize
new statistics-guzzle-1867382-9.patch8.38 KB

Patch rerolled.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new statistics-guzzle-1867382-10.patch8.29 KB

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

yesct’s picture

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 🇪🇺🌍
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.