Add README.txt to Bartik theme

Okay Ill update the README.txt shortly.

Could you explain what you'd propose to add in bullet form and why it would help in the issue summary?

Would make sense to closely follow https://www.drupal.org/node/2725039#comment-11395903.
Suggestions there closely follow READMEs in classy, stable and stark.

Created the patch for 8.3.x

@neerajpandey Thanks for picking this up. I would suggest your the following changes

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

   There should be a blank line at the end.

@denutkarsh Fixed.

@kifah-meeran Looks Good, but it's best practice to attach the interdiff with your new patch.

@denutkarsh Yes, I agree, but I think it's pointless for this case. I only added an extra line :) Can we RTBTC this?

Unassigning this because of inactivity from the assigned user and also because this needs review.

Great!

+++ b/core/themes/bartik/README.txt
@@ -0,0 +1,37 @@
+Bartik 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.
+
+It is a clean, safe, and stable theme with minimal colors and branding that can be easily used to administer Drupal.
+
...
+You may want to use your main theme for users adding and editing content. You can easily change this with the following steps:
...
+2) Scroll to the bottom to the "Administration Theme" section
+3) Check or Uncheck: Use the administration theme when editing or creating content
+4) Save Configuration

README's and such must be wrapped to 80 columns.

@cilefen Fixed 80 cols violations in this patch and even removed some lines from README.txt.

80 cols violation was Fixe by denutkarsh.
interdiff.txt is attached.

Bartik is the default administration theme for core in Drupal 8

That's not true ?!

Well, that surprisingly passed under my nose. Here we go, a new patch with the description of Seven Bartik...

Looks good to me.

Let us have the Bartik maintainer review it.

@kifah-meeran There is something wrong with the patch you submitted.

--- /dev/null
+++ b/core/themes/bartik/README.txt

Ah yes, something went wrong when editing the patch. Here it is again with the interdiff (between 20 and 31, since 23 failed completely).

+++ b/core/themes/bartik/README.txt
@@ -0,0 +1,37 @@
+3) Check or Uncheck: Use the administration theme when editing or creating content

80 cols violation

Thanks for the review. There is no 80 cols violation or wrong theme description now :). Patch and interdiff attached.

+++ b/core/themes/bartik/README.txt
@@ -0,0 +1,38 @@
+3) Check or Uncheck: Use the administration theme when editing or creating
+content

I think 'content' should be indented at the same level as 'Check'

Thanks @gvso, I have made the changes requested in #34

Whoops, forgot to attach patch.

Patch works as expected :-)

+++ b/core/themes/bartik/README.txt
@@ -0,0 +1,38 @@
+CHANGING ADMINISTRATIVE THEME
+-----------------------------
+
+1) Navigate to: /admin/appearance
+2) Scroll to the bottom to find the "Administration Theme" section
+3) Choose your theme from the selector
+4) Save Configuration
+
+CHANGING THEME USED FOR ADDING / EDITING CONTENT
+------------------------------------------------
+
+You may want to use your main theme for users adding and editing content.
+You can easily change this with the following steps:
+
+Changing the Theme used for adding and editing content,
+1) Navigate to: /admin/appearance
+2) Scroll to the bottom to the "Administration Theme" section
+3) Check or Uncheck: Use the administration theme when editing or creating
+   content
+4) Save Configuration

What do these instructions have to do with Bartik? None of the other theme README's include this.

I do agree with @cilefen. We should follow the Standard of how the other core themes have.

Reviewing this as part of SprintWeekend2017

Reviewed the README file in the patch and the extraneous information not found in the other theme's README files has been removed.

I will note that the Issue Summary on this ticket did ask to have that added = "Add a README.txt for the bartik theme that includes instructions on changing Admin theme and changing theme for add / edit node pages."

#5, "why are we doing this?" was never really answered. I guess there is a movement afoot to add READMEs to all the themes. I created a meta issue for that, since this seems a kind of standard that we add READMEs to themes in core now.

I removed from the issue summary the goals of documenting how to change the administration and add/edit theme because these notions are not specific to Bartik. So as per #5, what are the goals of this issue? The current patch does the following things:

• Repeat what is in the theme info file.
• Explain the origin of the theme's name.
• Link to documentation specific to the theme.
• Link to documentation about Drupal theming.

If that is really the general idea, somebody please update the meta issue.

NR for #44.

For what it's worth, I think the updated README documentation on this theme looks really great! Concise, clear, and informative. It even answers the question people always have about "what is a bartik". :)

Bartik and Seven are also the only themes that don't have a README:

./core/themes/classy/README.txt
./core/themes/stable/README.txt
./core/themes/stark/README.txt

This might partly be because the other themes are being used as base themes, whereas Bartik and Seven are not as intended to be extended. I think it might be worth adding a one-liner mentioning the base themes as a better starting point for new themes. What do you think?

Created a patch.

This is the patch.

Setting it to needs review to run the tests.

I've updated the issue as per #44.

@kleog, your patches failed because are a diff against a non existing README.txt file: error: core/themes/bartik/README.txt: No such file or directory. See lines 3 and 4 of your patches:

--- a/core/themes/bartik/README.txt
+++ b/core/themes/bartik/README.txt

You made also other mistakes in your patch: no new line at the end of the file and a whitespace at the end of the line ABOUT BARTIK.
Please (re-)read the guide at https://www.drupal.org/node/1319154

By the way, the patch at #41 already fixes the issue.
If somebody else agrees, please update the issue status.

@xjm: maybe could be better to mention that on base themes! Isn't it? ;)
https://www.drupal.org/docs/8/theming-drupal-8/creating-a-drupal-8-sub-t...

Here is the patch with corrections

here is the patch for the issue.

Hi Kleog,

I have reviewed the patch and it seems to be the statements are incorrect. "Bartik is the default administration theme in Drupal8 core. It is defaul theme for all admin Drupal pages.". Bartik is not an administration theme and theming guide URL refers Drupal 7 theming guide "https://www.drupal.org/docs/7/theming". I have created a new patch with the fixes.

Please let me know if there are any updates.

Hi @kleog and @SenthilMohith,

as I've already stated in my previous comment #53, this issue was already fixed by this patch, posted by @rakesh.gectcr in comment #41.

Remember that every patch should be published alongside the interdiff file.

Hi Luigi Abbamonte (gi.ab),

Thanks for the updates. I agree with the patch #41.

Seems like the majority agree that patch #41 works best. I really like it as well. Also happy that I learned about why Bartik is called Bartik. For what it's worth, I'll also just note that patch #41 applies fine. :)

From #45

I think it might be worth adding a one-liner mentioning the base themes as a better starting point for new themes. What do you think?

I don't think this was addressed?

Also happy that I learned about why Bartik is called Bartik

Explain the origin of the theme's name.

I don't see that in the latest patch?

@larowlan see patch #41!

Hiding everything other than #41 to avoid confusion. @kleog, please don't provide patches when there is no work needed.

The patch looks good to me.

I think it might be worth adding a one-liner mentioning the base themes as a better starting point for new themes. What do you think?

from #45 still isn't addressed in the patch at #41

Created a new patch on #41.
Added comment given by @larowlan.
Please review.

The comment needs to indicate that bartik is not suitable as a base theme - see comment #45

A problem with saying that Bartik should not be used as a base theme in the README is that it would conflict with the example of sub-theming here: https://www.drupal.org/docs/7/theming/creating-a-sub-theme ...where Bartik is used as an example. Change the 'Creating a sub-theme' doc page also?

please review the patch

Hi I reviewed the patch #41.

Thanks for the patch. Providing interdiff of patch #75 and #71

Hello,

The patch needs little fixes. I have attached the snapshot of it and also the updated patch.

Thanks

+++ b/core/themes/bartik/README.txt
@@ -0,0 +1,21 @@
+See https://www.drupal.org/docs/8/theming-drupal-8/creating-a-drupal-8-sub-theme-or-sub-theme-of-sub-theme
+for creating a sub-theme in Drupal 8.

As mentioned by @xjm, in #45 that Bartik theme is not intended to be used as a base theme. This needs to be mentioned in the README file somewhere.

+++ b/core/themes/bartik/README.txt
@@ -0,0 +1,21 @@
+Bartik is a flexible, re-colorable theme with many regions and a responsive,

`Bartik is a flexible, re-colorable theme with many regions and a responsive,
mobile-first layout and is the default theme for Drupal.` to
`Bartik is the default theme for Drupal. Bartik is a flexible, re-colorable theme with 16 regions layout which is responsive and
mobile-first.`

This patch will solve the purpose

I'm working on this as part of the Nashville 2018 sprint.

We have re-rolled the patch with these changes.

• Added definite article, 'the' to sentence.
• Removed numbers to be consistent with other Readme's.
• Ensured character limits are below 80 per line.
• Removed hyphen from 're-colorable'.

I applied this patch to a clean install of 8.6.x and it applied cleanly
also ran this through code sniffer against drupal standards and this passed also

I've also applied the patch in #84successfully and reviewed the wording. I see nothing amiss.

attaching the innerdiff for patch #84

First of all, thanks all for working on this.

@ecrown: that's a full patch not an interdiff.

The current patch still does not mention that Bartik is not intended as a base theme (despite two core committers asking repeatedly, I am the third).

Apart of that this did not read as clean English to me:

Bartik is the default theme for Drupal. Bartik is a flexible, recolorable theme with 16 regions layout which is responsive and mobile-first layout and is the default theme for Drupal.

1. It repeats Bartik twice very close (not nice, but not a grammar problem)
2. "with 16 regions layout" does not seem like correct English
3. "which is responsive and mobile-first layout" is missing a word
4. The combination of the prior two in the same sentence looks like words overran
5. It says in a sentence at the beiginning and then again at the end that it is the default theme

Packing the same info in much less words without repetition:

Bartik is the default theme for Drupal. It is a flexible, recolorable theme with a responsive and mobile-first layout supporting 16 regions.

The info on themes at the end has this subtheming guide link without any apparent reason. Also the link to the theming guide is not the same URL as in all other 3 themes in core. Why?

Hi, I have included everything according to review comments, adding a patch here with all possible and required changes. Please review and give your valuable feedback.

Again adding new patch with correction. Please review this one.

Adding new patch with all changes. Please review it.

I have applied patch. After applying patch readme.txt file created.

Testbot Snafu.

should finally be okay.

Thanks all, committed! Also marked #2968744: Add README to Bartik Theme a duplicate and credited its contributors.