Closed (fixed)
Project:
Drupal core
Version:
8.6.x-dev
Component:
forum.module
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
27 Jun 2017 at 17:24 UTC
Updated:
2 Apr 2018 at 22:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
megan_m commentedAnd here is a patch to add the original topics data to the
template_preprocess_forumsvariablesComment #3
larowlanFine by me
Comment #4
lauriiiRe-assigning variables is one of the anti-patterns that core does everywhere so I agree that we can add this. Maybe we should create a follow-up to remove all of the re-assignments of variables in preprocess functions, and maybe make that even a coding standard?
Anyway, setting to needs work still since we should document the newly created variable in the templates.
Comment #5
megan_m commentedHere is a new patch with documentation added to the forums template. Is this the right place to add it? Is there any place else I should document this? Is the description okay?
Comment #6
larowlanThanks again
Comment #7
lauriiiSorry but I didn't realize this on my first review, but we do need a super small change record for this. This would be a great new contributor task so here are instructions how to do that.
Comment #8
vegantriathleteSee @lauriii's suggestion in comment #7. Also creating an issue summary would be helpful.
Comment #10
megan_m commentedComment #11
megan_m commentedDraft change record added.
Comment #12
vegantriathleteComment #14
andypostClosed as duplicate #2953935: Allow to use original $variables['topics'] data in TMEMENAME_preprocess_forums
Comment #15
dillix commented@megan_m can you reroll patch for 8.5.x? Then I can test it on our sites.
Comment #16
borisson_This patch still applies, I don't understand #15. There is a small offset in the patch - but that shouldn't be a problem. There is a change record, but I didn't really understand the change record untill I read the patch, so I updated that.
I think that means I'm not allowed to RTBC this.
Comment #17
andypostChange record is much better now, here's a re-roll of #5 patch just without offset
Comment #18
dillix commentedThanks @andypost. Patch applies successfylly for D8.4, D8.5 & D8.6
Comment #20
larowlanCommitted as ac66ac9 and pushed to 8.6.x. Thanks
Given @andypost and @dillix have been working on a few forum issues during KharkivGSW18, tagging it as such.
Please remove if that is incorrect.
Would love to see you all over in #2207263: Try and build /forum and /forum/{tid} with views
Comment #21
dillix commented@larowlan can you commit this to 8.5.x-dev? #17 applies fine and work with it as expected.
Comment #22
larowlanHi @dillix
As per http://drupal.org/core/d8-allowed-changes - feature requests can only be committed to the non-stable branch.
If a case could be made that this was a bug then it would be eligible.
Lee