Issue #484: Added comment blocks to be more complete with arg specs et-al fo… #3113
Conversation
…r standards compliance
|
Related to: backdrop/backdrop-issues#484 |
|
Website: http://3113.backdrop.backdrop.qa.backdropcms.org |
|
@ps23Rick these changes look great! I've added some comments about docs that need to be updated (I suspect they were incorrect before this PR) and provided some options for alternate wording in one case... What do you think of the options? |
|
Hey @jenlampton.. I'm assuming you are referring to updates to the original issue#484.. It looks like the original post was updated since I looked at it yesterday/last night. It looks good. If you'd like me to tackle more, I'm assuming you'll want something along the lines of a separate PR for maybe each directory or some other grouping that isn't obvious in my head at the moment. There are 181 instances of |
I left them as comments here, on the PR. See above? Hmm.. why do they say pending... I'll work on that...
Yep! The separate-PRs idea was primarily to keep you sane while working on them, so do whatever division feels reasonable to you :) |
| * | ||
| * @param $variables | ||
| * An array containing (but not limited to) the following: | ||
| * - css: The array of CSS files to be used for this page. |
There was a problem hiding this comment.
I don't think there is a $css variable in Backdrop. We may need to fix the docs, these look like they are from Drupal 7... Can you check template_preprocess_page() in system.module?
There was a problem hiding this comment.
Ok.. I'm not sure how to proceed on this.. I assumed the comments in page.tpl.php were accurate but as I well know in coding that is no guarantee. I found the offending template_preprocess_page() method located in ./core/includes/theme.inc. I didn't find any reference in system.module. Unfortunately without me having more knowledge of this body of code I'm not sure how to adjust this.
| * | ||
| * @param $variables | ||
| * An array containing (but not limited to) the following: | ||
| * - css: The array of CSS files to be used for this page. |
There was a problem hiding this comment.
same here... Backdrop calls backdrop_get_css() in the template file. I don't think this variable exists before then since we have no process layer to alter it, so if used, this variable might print an incomplete set of CSS files.
Fixes backdrop/backdrop-issues#484 (partial)
Initial rundown with alternate comment blocks. Seeking feedback on whether to continue or if further alterations are necessary/wanted.