Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Meta issue: #1843738: [meta] Convert views module to Twig
#1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch
Steps to test:
1. Create a few articles with title and body
2. Visit rss.xml
Profiling results
=== views-view-row-rss-8.x..views-view-row-rss-8.x compared (51990c6b6ce43..51990d1de2dc4):
ct : 63,239|63,239|0|0.0%
wt : 301,459|301,096|-363|-0.1%
cpu : 285,428|285,449|21|0.0%
mu : 13,096,408|13,096,408|0|0.0%
pmu : 13,230,208|13,230,208|0|0.0%
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...
=== views-view-row-rss-8.x..views-view-row-rss-1843758-34 compared (51990c6b6ce43..51990ceb6ce00):
ct : 63,239|63,540|301|0.5%
wt : 301,459|301,009|-450|-0.1%
cpu : 285,428|284,199|-1,229|-0.4%
mu : 13,096,408|13,126,136|29,728|0.2%
pmu : 13,230,208|13,259,232|29,024|0.2%
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...
Comment | File | Size | Author |
---|---|---|---|
#34 | interdiff_5530.txt | 788 bytes | joelpittet |
#34 | 1843758-29-views-view-row-rss.patch | 2.2 KB | joelpittet |
#33 | 1843760-33-twig-views-view-rss.patch | 3.85 KB | joelpittet |
#33 | interdiff.txt | 2.52 KB | joelpittet |
#29 | 1843758-29-views-view-row-rss.patch | 2.2 KB | joelpittet |
Comments
Comment #1
joelpittetComment #2
mbrett5062 CreditAttribution: mbrett5062 commentedTagging.
Comment #3
jpamental CreditAttribution: jpamental commentedConverted to core patch. Removes views-view-row-rss.tpl.php, adds views-view-row-rss.html.twig
Comment #4
jpamental CreditAttribution: jpamental commentedReassigning to Drupal core.
Comment #5
joelpittetAdding twig tag
Comment #6
joelpittetComment #7
dawehnerThis looks pretty solid.
Comment #8
joelpittetDid some Doc Cleanup to standards.
And replaced $vars with $variables in the preprocess.
There are some @todos for the variables that are going into the preprocess.
Comment #9
dawehnerLet's drop the unneeded spaces here, as well as the & references.
Comment #10
joelpittet@dawehner good call. Changing this to needs work for the cleanup.
Comment #11
star-szrTagging.
Comment #12
joelpittetCleaned this up bit, reverted the $variables bit so it's easier to read the conversion changes.
Comment #14
star-szr#12: twig-views-view-row-rss-1843758-12.patch queued for re-testing.
Comment #15
thedavidmeister CreditAttribution: thedavidmeister commentedNitpicks for #12:
+ * Default theme implementation to display a item in an views RSS feed.
"a" not "an" and "views" needs a capital V.
+ * Prepares variables for all RSS rows.
Maybe something like "Prepares variables for Views RSS item templates."
This issue summary needs some steps written for manual testing and somebody to actually do the testing.
Comment #16
star-szrCreating a revised patch and interdiff based on #15 would be a good novice task, tagging.
If you'd like to work on this, please assign the issue to yourself. If you get stuck, drop by #drupal-twig on IRC or post your questions here. Thanks!
Comment #17
tlattimore CreditAttribution: tlattimore commentedHere is a patch with the doc updates requested in #15.
Comment #18
tlattimore CreditAttribution: tlattimore commentedblast. Forgot to tag for review.
Comment #19
joelpittet@tlattimore Nice work. Could we get someone to do some manual testing and possibly someone from VDC to fill in the 3 @todo doc stuff?
Comment #20
dawehnerThe & is not needed, because we do have objects here (views has been written fro php4 comp ability back in d6).
Comment #21
star-szrCNW for #20 and changing the @param to be vars instead of variables for now:
Comment #22
joelpittet@dawehner removed the variables that weren't used at all and &refs and documented 'row'.
Fixed a small typo too on the twig file.
Comment #24
star-szr#22: 1843758-22-views-view-row-rss.patch queued for re-testing.
Comment #25
thedavidmeister CreditAttribution: thedavidmeister commentednitpick:
+ * - item_elements: RSS item elements (pubDate, creator, guid).
When we have an item with properties in a Twig template we usually document it like:
- elements: Summary about elements. Each element contains:
- foo: summary of element.foo
- bar: summary of element.bar
Comment #26
joelpittetHow about:
* - item_elements: RSS item elements rendered as XML (pubDate, creator, guid).
?As this is using a format_xml_elements() to get build the rendered output.
Comment #27
thedavidmeister CreditAttribution: thedavidmeister commented#26 - yeah, IMO that little bit of extra clarification is better. I didn't realise that these elements were XML and not properties of item_elements, so I'd bet an end-user could be similarly confused.
Comment #28
star-szrTagging for profiling.
Comment #29
joelpittetComment #30
thedavidmeister CreditAttribution: thedavidmeister commentedi'll have a shot at manual testing this as i think the code is neat now :)
Comment #30.0
thedavidmeister CreditAttribution: thedavidmeister commentedUpdated issue summary.
Comment #31
thedavidmeister CreditAttribution: thedavidmeister commentedHEAD: whitespace normalized for easy diffing:
with patch #29, whitespace normalized again:
Comment #32
thedavidmeister CreditAttribution: thedavidmeister commentedComment #33
joelpittetDoc updates, missing preprocess docs. Removed $options because they weren't being used and =& references.
Comment #34
joelpittetWhoops, wrong one. Back to #29
Comment #35
star-szrDibs on this for profiling.
Comment #36
star-szrAdded this to template_preprocess_views_view_rows_rss() to isolate the profiling to only this template conversion (not comparing loading Twig itself):
Benchmarked /rss.xml.
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...
Comment #36.0
star-szradded testing steps
Comment #37
star-szrRe-ran with 1000 loops instead of 100 to confirm:
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519a6b3fa9c9c&...
http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519a6b3fa9c9c&...
Comment #38
alexpott+1. Ready for #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch
Comment #39
alexpottCommitted 7016229 and pushed to 8.x. Thanks!
Comment #40.0
(not verified) CreditAttribution: commentedAdd profiling results