Add a README.txt to the Seven theme.

CommentFileSizeAuthor
#52 interdiff-2725039-50-52.txt500 bytesandrewmacpherson
#52 readme_file_seven_theme-2725039-52.patch858 bytesandrewmacpherson
#51 readme_file_seven_theme-2725039-50.patch855 bytesmanish-31
#48 interdiff_42-48.txt449 bytesbrentg
#48 readme_file_seven_theme-2725039-48.patch854 bytesbrentg
#44 afterpatch-readme-file.png22.83 KBbandanasharma
#43 readme_file_seven_theme-2725039-42.patch724 bytessurbz
#38 seven-theme-readmetxt-file.jpg196.97 KBmttsmmrssprks
#34 readme_file_seven_theme-2725039-34.patch746 bytesbrentg
#29 readme_file_seven_theme-2725039-29.patch684 bytesmayurjadhav
#25 2725039-25.patch632 bytesneha.gangwar
#25 2725039-25.patch632 bytesneha.gangwar
#23 interdiff-2725039-19-21.txt1.81 KBrajeshwari10
#23 2725039-23.patch1.77 KBrajeshwari10
#21 interdiff-2725039-19-21.txt1.3 KBrajeshwari10
#21 2725039-21.patch1.77 KBrajeshwari10
#19 README_added-2725039-19.patch1.8 KBjoginderpc
#16 2725039-16.patch1.8 KBpashupathi nath gajawada
#14 add_readme_txt_seven-2725039-14.patch1.78 KBpashupathi nath gajawada
#8 interdiff-readme.txt-3-8.txt480 bytesrajeshwari10
#8 added_new_line-2725039-8.patch1.87 KBrajeshwari10
#5 drupal-seven-readme.patch754 bytesNikhilesh Gupta
#3 add_readme_txt_to_seven-2725039-3.patch1.9 KBkristindev
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kristink2 created an issue. See original summary.

kristindev’s picture

kristindev CreditAttribution: kristindev at Hook 42 commented

I'm working on this today at DrupalCon Sprints

kristindev’s picture

kristindev CreditAttribution: kristindev at Hook 42 commented
Status: Active » Needs review
FileSize
add_readme_txt_to_seven-2725039-3.patch1.9 KB

I attached the patch with the README.txt file.

Status: Needs review » Needs work

The last submitted patch, 3: add_readme_txt_to_seven-2725039-3.patch, failed testing.

Nikhilesh Gupta’s picture

Nikhilesh Gupta CreditAttribution: Nikhilesh Gupta as a volunteer and at Melity commented
Status: Needs work » Needs review
FileSize
drupal-seven-readme.patch754 bytes

Attached is the patch with the README.txt file.

surbz’s picture

surbz CreditAttribution: surbz at Srijan | A Material+ Company commented
Version: 8.2.x-dev » 7.x-dev
Status: Needs review » Needs work 
+++ b/core/themes/seven/README.txt
@@ -0,0 +1,41 @@
\ No newline at end of file

Patch #3 applies clean.README file has good information.
There should be an extra new line at the end.
It would be good if we could add a README.txt to Drupal 7 also.

surbz’s picture

surbz CreditAttribution: surbz at Srijan | A Material+ Company commented
Version: 7.x-dev » 8.2.x-dev
rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
Assigned: Unassigned » rajeshwari10
Status: Needs work » Needs review
FileSize
added_new_line-2725039-8.patch1.87 KB
interdiff-readme.txt-3-8.txt480 bytes

Added new line at the end of code.

Thanks!!

joginderpc’s picture

joginderpc CreditAttribution: joginderpc at Srijan | A Material+ Company commented
Assigned: rajeshwari10 » Unassigned
Status: Needs review » Reviewed & tested by the community

Given information in README.txt file is nice and patch works for me passing this.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: added_new_line-2725039-8.patch, failed testing.

rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
Status: Needs work » Needs review

The test is passing in PHP 5.6 & MySQL 5.5 18,519 pass.

Thanks!!

joginderpc’s picture

joginderpc CreditAttribution: joginderpc at Srijan | A Material+ Company commented
Status: Needs review » Reviewed & tested by the community

Adding this as reviewed because the patch is passing in PHP 5.6 & MySQL 5.5 18,519.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Assigned: Unassigned » star-szr
Status: Reviewed & tested by the community » Needs review

I'm not sure about the amount of information in this file. For example, explaining sub theming seems OTT - also seven is marked @internal so encouraging sub themes seems a bad idea.

Assigning to Cottser for review.

pashupathi nath gajawada’s picture

pashupathi nath gajawada CreditAttribution: pashupathi nath gajawada as a volunteer and at Melity commented
FileSize
add_readme_txt_seven-2725039-14.patch1.78 KB

Hi,
Please find the updated patch which contains the README.txt file for the seven theme of Drupal 8.

Thanks,

Status: Needs review » Needs work

The last submitted patch, 14: add_readme_txt_seven-2725039-14.patch, failed testing.

pashupathi nath gajawada’s picture

pashupathi nath gajawada CreditAttribution: pashupathi nath gajawada as a volunteer and at Melity commented
Status: Needs work » Needs review
FileSize
2725039-16.patch1.8 KB

Plesae find the updated patch 16.

Status: Needs review » Needs work

The last submitted patch, 16: 2725039-16.patch, failed testing.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Title: Add README.txt to Seven Theme » Add README.txt to Seven theme
Issue summary: View changes
Related issues: +#2749901: Add README.txt to Bartik theme
joginderpc’s picture

joginderpc CreditAttribution: joginderpc at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
README_added-2725039-19.patch1.8 KB

Adding fresh patch Please review this if it got passes :) ...

cilefen’s picture

cilefen CreditAttribution: cilefen commented

There are some layout issues:

  1. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,42 @@
+Seven is the default administration theme for core in Drupal 8. This theme appears to
+those administering Drupal on all admin pages and by default to those adding the
+content.
+

    This paragraph exceeds 80 columns. 

  2. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,42 @@
+See https://www.drupal.org/theme-guide/8 for more information on Drupal 8 theming.

    So does this one. 

  3. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,42 @@
\ No newline at end of file

    There is no newline at the end of the file.

rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
FileSize
2725039-21.patch1.77 KB
interdiff-2725039-19-21.txt1.3 KB

adding patch with following changes said in # 20

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Noticed there's an extra return line after the first two headers but not after the bottom ones.

rajeshwari10’s picture

rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented
FileSize
2725039-23.patch1.77 KB
interdiff-2725039-19-21.txt1.81 KB

Added an extra return line at the bottom ones.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr as a volunteer and at Northern Commerce commented
Assigned: star-szr » Unassigned
Priority: Minor » Normal
Status: Needs review » Needs work

Thanks for the work so far everyone.

  1. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,45 @@
+appears tothose administering Drupal on all admin pages and by default to those
+adding the content.

    "tothose" - typo. But it's also very passive voice, what about something more along the lines of "This theme is used on all admin pages and by default on node edit forms." 

  2. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,45 @@
+It is a clean, safe, and stable theme with minimal colors and branding that can

    If it's internal is it really also stable? It can change. 

  3. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,45 @@
+be easily used to administer Drupal.

    Not sure about the "easily used to administer Drupal" part, maybe we can replace stable with usable if we think that's one of its qualities. 

  4. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,45 @@
+To read more about the Seven theme's origins (in D7) please see:

    D7 is jargon, let's at least replace this with Drupal 7. 

  5. +++ b/core/themes/seven/README.txt
@@ -0,0 +1,45 @@
+ADDING A SUBTHEME TO SEVEN

    I agree with @alexpott we should not describe how to create a Seven subtheme because it's actively discouraged.

Overall I'm not sure about including all this in the README, I think it'd be preferable to ensure we have good documentation for these points that are not Seven-specific on drupal.org and link to them rather than including them here.

neha.gangwar’s picture

neha.gangwar CreditAttribution: neha.gangwar as a volunteer commented
FileSize
2725039-25.patch632 bytes
2725039-25.patch632 bytes

As per the Cottser comments, i have made the readme file small and simple.

kostyashupenko’s picture

kostyashupenko
Primary language Russian
Location Omsk
CreditAttribution: kostyashupenko at Skilld commented
Status: Needs work » Needs review

The last submitted patch, 25: 2725039-25.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 25: 2725039-25.patch, failed testing.

mayurjadhav’s picture

mayurjadhav CreditAttribution: mayurjadhav at Blisstering Solutions commented
Status: Needs work » Needs review
FileSize
readme_file_seven_theme-2725039-29.patch684 bytes

Made the changes as per Cottser suggestion in #24.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Parent issue: » #2847879: [meta] Add READMEs to all themes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
Status: Needs review » Needs work

Like I wrote in #2749901: Add README.txt to Bartik theme, I don't see why we would want to document how to change the admin theme in the README for a specific theme. I've removed that from the issue summary.

+++ b/core/themes/seven/README.txt
@@ -0,0 +1,16 @@
+It is a clean, safe and usable theme with minimal colors and branding.

I have no idea what "safe" and "usable" mean here. Safe, for what purpose? Are other themes "unusable". Ha, maybe...

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg at Dropsolid commented
FileSize
readme_file_seven_theme-2725039-34.patch746 bytes

Removed the text from https://www.drupal.org/node/2725039#comment-11898607, also added a link to the theme page on drupal.org, like it is in Bartik

Applied the patch

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg at Dropsolid commented
Status: Needs work » Needs review

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mttsmmrssprks’s picture

mttsmmrssprks CreditAttribution: mttsmmrssprks commented

I'm working on this today at DrupalCon.

mttsmmrssprks’s picture

mttsmmrssprks CreditAttribution: mttsmmrssprks commented
FileSize
seven-theme-readmetxt-file.jpg196.97 KB

I've tested this. It's applied correctly.
Screenshot attached.

aburrows’s picture

aburrows CreditAttribution: aburrows as a volunteer commented
Status: Needs review » Reviewed & tested by the community

I mentored @mttsmmrssprks and saw the patch apply correctly and then re ran locally and it worked as intended as per screenshot.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs review

Thanks @aburrows and @mttsmmrssprks!

In this case, a screenshot of applying the patch isn't needed. The automated testing infrastructure (a.k.a. "testbot") checks for us whether or not the patch applies. For this issue, let's review the content of the text added, and see if it seems complete and correct. We can also compare it to other READMEs that have been added.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented 
+++ b/core/themes/seven/README.txt
@@ -0,0 +1,17 @@
+See https://www.drupal.org/theme-guide/8 for more information on Drupal 8
+theming.
\ No newline at end of file

Oh, also! Let's make sure to add a single newline to the end of the file to comply with our coding standards. Thanks!

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

surbz’s picture

surbz CreditAttribution: surbz at Srijan | A Material+ Company commented
FileSize
readme_file_seven_theme-2725039-42.patch724 bytes

Thanks @aburrows and @mttsmmrssprks! for this patch.
Thanks @xjm for reviewing this patch I have addressed #40 #41 and #42 and readme_file_seven_theme-2725039-42.patch looks final and complete and is ready for review.
Comparing this README to READMEs that have been added this content looks good.

bandanasharma’s picture

bandanasharma CreditAttribution: bandanasharma as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
Status: Needs review » Reviewed & tested by the community
FileSize
afterpatch-readme-file.png22.83 KB

I have tested the #43 patch and it's applied cleanly. Attached the screen shot.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 43: readme_file_seven_theme-2725039-42.patch, failed testing. View results

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Needs work » Reviewed & tested by the community

Testbot Snafu.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs review

Should we mention in the README.txt that Seven is internal theme and shouldn't be extended by other themes?

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg at Dropsolid commented
Issue tags: +DevDaysLisbon
FileSize
readme_file_seven_theme-2725039-48.patch854 bytes
interdiff_42-48.txt449 bytes

Good suggestion @lauriii, I've added it to the patch including a link to the change record [#2582945]

Created a patch and interdiff

andrewmacpherson’s picture

andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented
Status: Needs review » Needs work

Patch in #48 addresses @lauriii's point from #47, that part's good.

The D8 theme-guide URL gets a 301 redirect though, i.e.
https://www.drupal.org/theme-guide/8

301 redirects to ...
https://www.drupal.org/docs/8/theming

The latter URL is the one mentioned by the README recently added to Bartik. I think this was likely part of the plan to re-organize the handbook on d.o

Can we update this URL in the Seven README too please?

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg at Dropsolid commented
Status: Needs work » Needs review
FileSize
readme_file_seven_theme-2725039-50.patch855 bytes
interdiff_48-50.txt321 bytes

Updated the patch with the changes suggested by Andrew.

manish-31’s picture

manish-31 CreditAttribution: manish-31 at OpenSense Labs for DrupalFit commented
Assigned: Unassigned » manish-31
FileSize
readme_file_seven_theme-2725039-50.patch855 bytes

Edited theme-guide URL in patch #48.
Thanks @andrewmacpherson for noticing it.

andrewmacpherson’s picture

andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented
FileSize
readme_file_seven_theme-2725039-52.patch858 bytes
interdiff-2725039-50-52.txt500 bytes

Thanks @brentgees and @manish-31. The patches in #50 and #51 are the same, and fix the redirected URL noted in #49.

There's another one though.

https://www.drupal.org/documentation/themes/seven.
redirects to:
https://www.drupal.org/docs/7/core/themes/seven

It looks like another one from reorganizing the handbooks. I've updated it in a patch here. Now all the URLs in the README are the current ones, avoiding redirects.

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Thanks @andrewmacpherson, I've noticed the duplicate in #50 and #51 as well, pretty funny two identical patches so short after each other :)

I think the file looks good now, I don't notice any mistakes anymore.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

Adjusting credits.

  • Gábor Hojtsy committed b91f5b4 on 8.6.x 
    Issue #2725039 by rajeshwari10, brentgees, neha.gangwar,...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed b91f5b4 and pushed to 8.6.x. Thanks!

Status: Fixed » Closed (fixed)

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