Replace theme() with drupal_render() in forum module [#2009574]

Comment	File	Size	Author
#29	twig-forum-2009574-29.patch	4.22 KB	e2tha-e
#24	twig-forum-2009574-24.patch	2.3 KB	e2tha-e
#22	twig-forum-2009574-22.patch	4.21 KB	e2tha-e
#20	twig-forum-2009574-20.patch	4.21 KB	e2tha-e
#18	twig-forum-2009574-18.patch	3.87 KB	e2tha-e
#16	twig-forum-2009574-16.patch	3.85 KB	e2tha-e
#14	twig-forum-2009574-14.patch	4.43 KB	jenlampton
#12	twig-2009574-12.patch	4.15 KB	adamcowboy
#10	interdiff.txt	2.23 KB	adamcowboy
#10	twig-2009574-9.patch	4.14 KB	adamcowboy
#8	twig-2009574-7.patch	4.15 KB	adamcowboy
#5	replace-theme-with-render-2009574-5.patch	4.14 KB	samvel
#2	replace-theme-in-forum-2009574-2.patch	4.06 KB	samvel

Comment #1

samvel commented 1 June 2013 at 05:42

Assigned:

Unassigned

» samvel

Log in or register to post comments

Comment #2

samvel commented 1 June 2013 at 16:49

Status:

Active

» Needs review

Status	File	Size
new	replace-theme-in-forum-2009574-2.patch	4.06 KB

attached

Log in or register to post comments

Comment #3

samvel commented 2 June 2013 at 10:14

Issue tags:

+CodeSprintUA

+ CodeSprintUA

Log in or register to post comments

Comment #4

thedavidmeister commented 2 June 2013 at 13:20

Status:

Needs review

» Needs work

A few of the renderable arrays go past 80 characters. Could they be split over multiple lines please?

Log in or register to post comments

Comment #5

samvel commented 3 June 2013 at 18:03

Status:

Needs work

» Needs review

Status	File	Size
new	replace-theme-with-render-2009574-5.patch	4.14 KB

I found 1 critical, 2 normal and 1 low places.
Critical and normal fixed.

Attached new one.

Log in or register to post comments

Comment #6

thedavidmeister commented 16 June 2013 at 04:24

Status:

Needs review

» Needs work

+    $build = array('#theme' => 'forum_submitted', '#topic' => $forum->last_post);
+      $created = array('#theme' => 'forum_submitted', '#topic' => $topic);
+    $build = array('#theme' => 'username', '#account' => $variables['topic']);
+    $breadcrumb_build = array(
+    $breadcrumb_build = array(

These variables should all be the name of the #theme, eg. use $forum_submitted instead of $build. The only time you don't want to do this is when it would override the value of another variable already in the current scope.

See the issue summary of the parent issue for details.

Log in or register to post comments

Comment #7

adamcowboy commented 20 June 2013 at 21:59

Status:

Needs work

» Needs review

I tried to make the changes.

Log in or register to post comments

Comment #8

adamcowboy commented 20 June 2013 at 21:59

Status	File	Size
new	twig-2009574-7.patch	4.15 KB

Log in or register to post comments

Comment #9

20 June 2013 at 23:13

Status:

Needs review

» Needs work

The last submitted patch, twig-2009574-7.patch, failed testing.

Log in or register to post comments

Comment #10

adamcowboy commented 25 June 2013 at 23:04

Status:

Needs work

» Needs review

Status	File	Size
new	twig-2009574-9.patch	4.14 KB
new	interdiff.txt	2.23 KB

I tried again :D. Interdiff included

Log in or register to post comments

Comment #11

26 June 2013 at 01:58

Status:

Needs review

» Needs work

The last submitted patch, twig-2009574-9.patch, failed testing.

Log in or register to post comments

Comment #12

adamcowboy commented 27 June 2013 at 23:15

Status:

Needs work

» Needs review

Status	File	Size
new	twig-2009574-12.patch	4.15 KB

Go Go Bot!

Log in or register to post comments

Comment #13

28 June 2013 at 05:01

Status:

Needs review

» Needs work

The last submitted patch, twig-2009574-12.patch, failed testing.

Log in or register to post comments

Comment #14

jenlampton

she / her

commented 2 July 2013 at 01:10

Status:	Needs work	» Needs review
Issue tags:	-CodeSprintUA

Status	File	Size
new	twig-forum-2009574-14.patch	4.43 KB

Rerolled. :)

Log in or register to post comments

Comment #15

e2tha-e commented 14 July 2013 at 19:56

Assigned:

samvel

» e2tha-e

I'm working on this

Log in or register to post comments

Comment #16

e2tha-e commented 14 July 2013 at 21:21

Status	File	Size
new	twig-forum-2009574-16.patch	3.85 KB

Rererolled :))

Log in or register to post comments

Comment #17

pplantinga commented 16 July 2013 at 21:50

Status:

Needs review

» Needs work

Surely we can come up with a better variable name than $crubms?

Log in or register to post comments

Comment #18

e2tha-e commented 20 July 2013 at 03:58

Status:

Needs work

» Needs review

Status	File	Size
new	twig-forum-2009574-18.patch	3.87 KB

The name of array $crubms has been changed to $breadcrumb. The $breadcrumb array is rolled into a render array named $breadcrumb_build as per the precedent set by the verifyForumView function in the same class ForumTest.

Log in or register to post comments

Comment #19

pplantinga commented 20 July 2013 at 07:02

Status:

Needs review

» Reviewed & tested by the community

Cool. Looks good to me!

Log in or register to post comments

Comment #20

e2tha-e commented 20 July 2013 at 10:43

Status:

Reviewed & tested by the community

» Needs review

Status	File	Size
new	twig-forum-2009574-20.patch	4.21 KB

Switched the names of the $breadcrumb and $breadcrumb_build arrays in functions verifyForums and verifyForumView function in class ForumTest. This is to abide by the naming convention requested in comment #6.