Problem/Motivation

Suggested commit message

please add AnninaJ

As I was working on a new Drupal 8 theme, I noticed there were no documentation about the update date of the node in the /core/modules/node/templates/node.html.twig file.

Proposed resolution

Update the core node twig template and apply a similar change to the core themes (bartik, seven)

 

 

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch X Instructions
Reroll the patch if it no longer applies. X Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards X Instructions

Remaining tasks

User interface changes

None

API changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug
Issue priority Minor because it affects only docblocks.
Unfrozen changes Unfrozen because it only changes docblocks.
Prioritized changes This is not a prioritized change for the beta phase.
Disruption Theres no disruption because we only change documentation.
Files: 
CommentFileSizeAuthor
#26 drupal_core-missing-documentation-update-date-2349503-26.patch2.7 KBmeramo
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,624 pass(es). View
#24 interdiff-22-24.txt2.76 KBtadityar
#24 drupal_core-missing-documentation-update-date-2349503-24.patch2.57 KBtadityar
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,408 pass(es). View
#22 drupal_core-missing-documentation-update-date-2349503-22.patch2.24 KBpiyuesh23
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,785 pass(es). View
#15 drupal_core-missing-documentation-update-date-2349503-15.patch2.24 KBjanne.valtakari
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,426 pass(es). View
#6 interdiff-2349503-3-6.txt699 bytesguntervs
#6 missing_documentation-2349503-6.patch1.57 KBguntervs
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,154 pass(es). View
#3 drupal-missing-documentation-update-date-2349503-3.patch801 bytestargoo
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,854 pass(es). View
#2 drupal-missing-documentation-update-date-2349503-3.patch801 bytestargoo
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,859 pass(es). View

Comments

targoo’s picture

Title: Missing documentation about the creation date properties in node template » Missing documentation about the creation date in node template
targoo’s picture

Title: Missing documentation about the creation date in node template » Missing documentation about the updated date in node template
Issue summary: View changes
FileSize
801 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,859 pass(es). View

attached patch to amend twig template documentation

targoo’s picture

FileSize
801 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,854 pass(es). View

reroll

targoo’s picture

Status: Active » Needs review

active >> need review

guntervs’s picture

Assigned: Unassigned » guntervs

Reviewing.

guntervs’s picture

FileSize
1.57 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,154 pass(es). View
699 bytes

Targoo, i've added the same documentation to the documentation of Bartik as well.

guntervs’s picture

Assigned: guntervs » Unassigned
targoo’s picture

cool. Thanks. We need to keep an eye on it as with template could move to the new classy "base" theme very soon.

alvar0hurtad0’s picture

Status: Needs review » Reviewed & tested by the community

Patch apply and works as #6 and #3 says.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/node/templates/node.html.twig
@@ -11,6 +11,9 @@
+ *   - changedtime: Formatted changed date. Preprocess functions can
+ *     reformat it by calling format_date() with the desired parameters on
+ *     $variables['node']->getChangedTime().

+++ b/core/themes/bartik/templates/node.html.twig
@@ -11,6 +11,9 @@
+ *   - changedtime: Formatted changed date. Preprocess functions can
+ *     reformat it by calling format_date() with the desired parameters on
+ *     $variables['node']->getChangedTime().

The bit about preprocess functions reformatting it looks false.

pwolanin’s picture

Issue summary: View changes
pwolanin’s picture

Issue summary: View changes
cilefen’s picture

pwolanin’s picture

Issue summary: View changes
janne.valtakari’s picture

Status: Needs work » Needs review
FileSize
2.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,426 pass(es). View

Janne to the rescue!

Fixed the row length and it now seems to run smooth as silk.

You are welcome.

Now it is time for other rescue missions. TBC

lauriii’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Looks good to me. Added AnninaJ also there since she was working on the issue with Janne at the Drupal sprints in Helsinki.

lauriii’s picture

Issue summary: View changes

Added beta evaluation

lauriii’s picture

Issue summary: View changes
YesCT’s picture

Issue tags: -Novice

1.
@lauriii thank you for working with new contributors.

I see the suggested commit message you added to the summary (#16).

For the message to be parsed correctly later, a comma needs to be between each drupal.org username.

Also, please contact AnninaJ and have them make a comment on this issue. Since they were working on it also, perhaps they can add some information in the form of a review, or upload their version of the patch they were working on making. (Which we could then use to compare to @janne.valtakari's as a kind of a review).

2.
AnninaJ, it can be frustrating to work on something, and when you come to an issue to post your work, you see someone else has just posted something similar.

When that happens, the "silver lining", is that you can probably review the work they did.

To prevent that in the future, when you start to do something on an issue, it is OK (and I even encourage people) to post a comment and say what you are about to do. Like "I'm at a sprint in Brazil with @YesCT and I'm going to reformat the comment to 80 characters. It will be my first core patch." or "I'm going to do manual testing on this issue now." or "I'm going to... whatever." you get the idea. It can also help to jump into irc in #drupal-contribute and say something similar.

3.
in #10 @alexpott said

The bit about preprocess functions reformatting it looks false.

I dont think Alex was worried that the 80 character format of the comment was off.

It sounds like he meant the comment was not accurate.

@lauriii you RTBC'd this, can you please share with us some reference or explanation at how you checked that the comment was accurate and how the change addressed @alexpott's concern? "Looks good to me" does not really express enough information for me to know what it was that you evaluated. Did you evaluate just the 80 character coding standard, or did you look into the accuracy of the comment itself and how it relates to @alexpott's concern? It's ok if you only did part, but it helps to know that some other part still needs review.

lauriii’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

1-2: Janne and Annina were working together on the issue at the sprints so there's only one ouput :)

3. I'm sorry, I misread @alexpott's comment and RTBC'd the issue against #9

Cottser’s picture

Title: Missing documentation about the updated date in node template » Incorrect documentation about dates in node template
Issue tags: +Novice, +Twig, +documentation

Before I forget: We need to update Classy's node.html.twig as well.

Also before I forget: Thanks to everyone who has worked on this so far :)

  1. +++ b/core/modules/node/templates/node.html.twig
    @@ -8,9 +8,12 @@
    - *   - createdtime: Formatted creation date. Preprocess functions can
    - *     reformat it by calling format_date() with the desired parameters on
    + *   - createdtime: Formatted creation date. Preprocess functions can reformat
    + *     it by calling format_date() with the desired parameters on
      *     $variables['node']->getCreatedTime().
    + *   - changedtime: Formatted changed date. Preprocess functions can reformat it
    + *     by calling format_date() with the desired parameters on
    + *     $variables['node']->getChangedTime().
    

    Neither node.createdtime nor node.changedtime are "formatted" dates, they both come out as Unix timestamps.

    So I think we should:

    1. Say that they are Unix timestamps.

    2. Remove the bit about reformatting in preprocess, that rings false to me because you can't manipulate a function call. git log -S led me to #2049039: Convert node properties to methods. on that point.

  2. +++ b/core/modules/node/templates/node.html.twig
    @@ -8,9 +8,12 @@
    diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh
    
    diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh
    old mode 100644
    new mode 100755
    

    Remove the file mode change from the patch.

piyuesh23’s picture

Status: Needs work » Needs review
Issue tags: +#drupalgoa2015
FileSize
2.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,785 pass(es). View

Re-rolled the patch and adding the updating patch.

Cottser’s picture

Status: Needs review » Needs work

Thanks @piyuesh23, please see comment 21. This didn't really need a reroll, a new patch will be easier IMO. But now we can do an interdiff I suppose :)

tadityar’s picture

FileSize
2.57 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,408 pass(es). View
2.76 KB

I wonder if the description is good enough..

tadityar’s picture

Status: Needs work » Needs review

Oops, forgot to change the status

meramo’s picture

Issue tags: +drupaldevdays
FileSize
2.7 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,624 pass(es). View

Updated the text to make it more descriptive. Otherwise looking good!

Reno Greenleaf’s picture

Updated the text to make it more descriptive.

Works fine for me.

chegor’s picture

Status: Needs review » Reviewed & tested by the community

Looks good

webchick’s picture

Issue tags: +LIVE commit candidate

I was going to commit this, but realized it might make a good candidate for a LIVE commit. :)

YesCT’s picture

Issue summary: View changes
YesCT’s picture

Issue tags: -LIVE commit candidate
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, we ended up doing a different patch on live commit, sorry about that!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 8063f77 on 8.0.x
    Issue #2349503 by targoo, guntervs, tadityar, meramo, piyuesh23, janne....

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.