Use Symfony Request for core classes [#1998700]

Comment	File	Size	Author
#26	1998700-core-includes-request-26.patch	4 KB	kim.pepper
#22	1998700-core-includes-request-22.patch	3.97 KB	kim.pepper
#19	1998700-core-includes-request-19.patch	3.97 KB	kim.pepper
#14	1998700-core-includes-request-14.patch	4.7 KB	kim.pepper
#14	interdiff.txt	6.1 KB	kim.pepper
#3	drupal-use-symfony-request-for-core-classes-1998700-3.patch	5.07 KB	mhagedon

Comment #1

mhagedon commented 24 May 2013 at 17:23

Assigned:

Unassigned

» mhagedon

Log in or register to post comments

Comment #2

atchijov commented 24 May 2013 at 18:30

sorry. have not see that it is taken.

Log in or register to post comments

Comment #3

mhagedon commented 24 May 2013 at 18:38

Status:

Active

» Needs review

Status	File	Size
new	drupal-use-symfony-request-for-core-classes-1998700-3.patch	5.07 KB

Here's an attempt to fix this. I used Drupal::request() a fair amount because it didn't appear that these classes were using DI. Suggestions to fix that are welcome. Also, AjaxResponse seemed clean of superglobals.

Log in or register to post comments

Comment #4

25 May 2013 at 00:13

Status:

Needs review

» Needs work

The last submitted patch, drupal-use-symfony-request-for-core-classes-1998700-3.patch, failed testing.

Log in or register to post comments

Comment #5

mhagedon commented 25 May 2013 at 00:54

Status:

Needs work

» Needs review

#3: drupal-use-symfony-request-for-core-classes-1998700-3.patch queued for re-testing.

Log in or register to post comments

Comment #6

25 May 2013 at 00:58

Status:

Needs review

» Needs work

The last submitted patch, drupal-use-symfony-request-for-core-classes-1998700-3.patch, failed testing.

Log in or register to post comments

Comment #7

atchijov commented 25 May 2013 at 19:08

Status:

Needs work

» Needs review

#3: drupal-use-symfony-request-for-core-classes-1998700-3.patch queued for re-testing.

Log in or register to post comments

Comment #8

25 May 2013 at 19:11

Status:

Needs review

» Needs work

The last submitted patch, drupal-use-symfony-request-for-core-classes-1998700-3.patch, failed testing.

Log in or register to post comments

Comment #9

atchijov commented 25 May 2013 at 20:59

Status:

Needs work

» Needs review

#3: drupal-use-symfony-request-for-core-classes-1998700-3.patch queued for re-testing.

Log in or register to post comments

Comment #10

25 May 2013 at 21:01

Status:

Needs review

» Needs work

The last submitted patch, drupal-use-symfony-request-for-core-classes-1998700-3.patch, failed testing.

Log in or register to post comments

Comment #11

atchijov commented 25 May 2013 at 22:01

Status:

Needs work

» Needs review

#3: drupal-use-symfony-request-for-core-classes-1998700-3.patch queued for re-testing.

Log in or register to post comments

Comment #12

25 May 2013 at 22:05

Status:

Needs review

» Needs work

The last submitted patch, drupal-use-symfony-request-for-core-classes-1998700-3.patch, failed testing.

Log in or register to post comments

Comment #13

kim.pepper

English

🏄‍♂️🇦🇺Sydney, Australia

commented 30 May 2013 at 10:50

Are you able to run these tests locally?

Log in or register to post comments

Comment #14

kim.pepper

English

🏄‍♂️🇦🇺Sydney, Australia

commented 1 June 2013 at 20:34

Status:

Needs work

» Needs review

Status	File	Size
new	interdiff.txt	6.1 KB
new	1998700-core-includes-request-14.patch	4.7 KB

Cleaned up the patch using $request->isMethod('GET') as well as removing some temporary comments.

Log in or register to post comments

Comment #15

1 June 2013 at 21:48

Status:

Needs review

» Needs work

The last submitted patch, 1998700-core-includes-request-14.patch, failed testing.

Log in or register to post comments

Comment #16

mhagedon commented 12 June 2013 at 17:00

Odd... the change to isMethod() (good call BTW!) shouldn't have made the testbot able to log into the site. Maybe it was just overwhelmed by the volume of testing coming from DrupalCon?

I was (am) brand new to Drupal core development, so I didn't know how to run the tests at the time. I'm still not quite sure how I'd isolate these particular changes...