Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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 |
Comments
Comment #1
Samvel CreditAttribution: Samvel commentedComment #2
Samvel CreditAttribution: Samvel commentedattached
Comment #3
Samvel CreditAttribution: Samvel commented+ CodeSprintUA
Comment #4
thedavidmeister CreditAttribution: thedavidmeister commentedA few of the renderable arrays go past 80 characters. Could they be split over multiple lines please?
Comment #5
Samvel CreditAttribution: Samvel commentedI found 1 critical, 2 normal and 1 low places.
Critical and normal fixed.
Attached new one.
Comment #6
thedavidmeister CreditAttribution: thedavidmeister commentedThese 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.
Comment #7
adamcowboy CreditAttribution: adamcowboy commentedI tried to make the changes.
Comment #8
adamcowboy CreditAttribution: adamcowboy commentedComment #10
adamcowboy CreditAttribution: adamcowboy commentedI tried again :D. Interdiff included
Comment #12
adamcowboy CreditAttribution: adamcowboy commentedGo Go Bot!
Comment #14
jenlamptonRerolled. :)
Comment #15
e2tha-e CreditAttribution: e2tha-e commentedI'm working on this
Comment #16
e2tha-e CreditAttribution: e2tha-e commentedRererolled :))
Comment #17
pplantinga CreditAttribution: pplantinga commentedSurely we can come up with a better variable name than
$crubms
?Comment #18
e2tha-e CreditAttribution: e2tha-e commentedThe 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.
Comment #19
pplantinga CreditAttribution: pplantinga commentedCool. Looks good to me!
Comment #20
e2tha-e CreditAttribution: e2tha-e commentedSwitched 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.
Comment #21
thedavidmeister CreditAttribution: thedavidmeister commented+ $breadcrumb= array(
Missing space after $breadcrumb.
After that, seems fine.
Comment #22
e2tha-e CreditAttribution: e2tha-e commentedSpacing fixed. Rerolled.
Comment #24
e2tha-e CreditAttribution: e2tha-e commentedPatch rerolled after pulling latest from git repo.
Comment #25
siccababes CreditAttribution: siccababes commentedI tested this patch by creating a forum topic and commenting. I also added the two blocks relevant to the forum. Everything looks good.
Comment #26
pplantinga CreditAttribution: pplantinga commentedWhat happened to changes in ForumTest.php? They seem to be missing from latest patch.
Comment #27
e2tha-e CreditAttribution: e2tha-e commentedcore/modules/forum/lib/Drupal/forum/Tests/ForumTest.php has been removed from the latest git revisions. I don't know anything more than that.
Comment #28
pplantinga CreditAttribution: pplantinga commentedActually I don't think it has:
https://api.drupal.org/api/drupal/core!modules!forum!lib!Drupal!forum!Te...
Check again. Have you accidentally checked in a change that removes that file? Do a clean
git reset --hard 8.x
orgit clone
Comment #29
e2tha-e CreditAttribution: e2tha-e commentedRerolled with correctly pulled git. Changes to ForumTest.php restored.
Comment #30
siccababes CreditAttribution: siccababes commentedI tested this patch by creating a forum topic and commenting on it. I also added the two blocks pertaining to the forum. Everything looked good.
Comment #31
xjm#29: twig-forum-2009574-29.patch queued for re-testing.
Comment #33
pplantinga CreditAttribution: pplantinga commented#29: twig-forum-2009574-29.patch queued for re-testing.
Comment #34
pplantinga CreditAttribution: pplantinga commentedLooks good and passes manual testing (#30)
Comment #35
webchickCommitted and pushed to 8.x. Thanks!