Add README.txt to Classy Base Theme?

For Read Me, why other files need to be modified?

I am not able to apply this patch, error details attached.

Now README.txt created, please review.

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,11 @@
   +Classy is a theme in Drupal 8 core that is used as a base theme for Bartik and Seven.
   

   Can this be broken into 2 lines so that line does not have more than 80 characters?

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,11 @@
   +Classy provides basic classes throughout Drupal's default markup. This can be useful to themers who want a starting point for their themes without having to add classes throughout templates, etc. Themers that want markup coming out of Drupal to be as clean as possible (no classes), can simply ignore classy.
   

   Same thing here..

Almost there :) Few nitpicks below

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,15 @@
   +Classy is a theme in Drupal 8 core that is used as a base theme for ¶
   

   Trailing space should be removed.

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,15 @@
   +Classy provides basic classes throughout Drupal's default markup. This can be ¶
   

   Trailing space again.

3. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,15 @@
   +useful to themers who want a starting point for their themes without having ¶
   

   Trailing space again. This is the last :)

Made the changes.

Sorry forgot to add patch.

Thanks @Chrisfree

@chrisfree,
Thanks for the patch. It looks fine to me.
It is RTBC now.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,15 @@
+Classy is a theme in Drupal 8 core that is used as a base theme for Bartik and
+Seven.

This might need some rewording. It seems to imply that Classy was added specifically to serve as a base for Bartik and Seven, which is not the case.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,15 @@
+to add classes throughout templates, etc. Themers that want markup coming out

This sentence needs work. The "etc" in particular should probably be removed.

In general, is the intention here to simply describe or provide some instruction. It would probably help to include information on how to use Classy, ie. adding the setting in the theme's info file. Also, instead of linking to the change record it is probably better to link to the drupal.org theming guide page. https://www.drupal.org/theme-guide/8/classy

Hi davidhernandez,
1. Classy is used as a base for Bartik and Seven. So its not wrong,but could we use something like this.

Classy is a theme in Drupal 8 core that is used as a base theme for Bartik and
Seven. It can also be used as independent theme.

Though it seems classy was created for this( https://www.drupal.org/node/2289511 ).

Made changes for #2 and provide link to theming guide page.

Agreed with @davidhernandez the fact that it's used by Bartik and Seven is not really the most relevant thing here. We should be describing what makes Classy different and why people would use it over Stable.

Agreed. We should lead off saying Classy is a good base theme if you want a standard set of classes added to your divs etc. and contrast that with Stable. Especially now that we have a README for Stable. (having just committed #2612618: Add README.txt file to Stable explain its role as a backwards compatibility layer)

Hi,

Please review the patch file I have uploaded. Thanks.

Hi,
Recreated patch and added interdiff file.

I am trying to make it a bit clearer (hopefully) and fixing some errors.

Small edit.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,19 @@
+to your markup. Classy is the base theme for Bartik and Seven core themes. To
+use Classy as the base theme in your theme, set the 'base theme' in your theme's
+.info.yml to 'classy':

as a base theme in your your theme,

The "in your theme" part seems to be redundant.

I think it could be removed, and just use

To use Classy as the base theme, set the 'base theme' in your theme's .info.yml to 'classy':

Good point.

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +use Classy as the base theme, set the 'base theme' in your theme's .info.yml to
   

   "use Classy as your base theme" would be better here I think.

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +markup to be as clean as possible (no classes) may simply ignore classy.
   

   IMO it might be nice here to:

   Perhaps be a bit clearer about what we mean by "ignore classy", and/or spell out that Stable is the alternative for cleaner markup, since it has its own README now. See #2630592: Tweak Stable's README.txt to be more understandable by new users where we're trying to mention Classy in Stable's README :)

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,19 @@
+markup to be as clean as possible (no classes) may simply ignore classy.

Also, "Classy" should be capitalized here.

Tried to correct the language.
Kindly review and suggest if any more changes required.

Thanks -- this is looking OK.... There's one grammatical nitpick:

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,20 @@
+markup to be as clean as possible (less classes) can use Stable, the default

less => fewer

Also... Hm... The first paragraph just seems to be a random unconnected list of statements -- it's choppy. And it starts to talk about classes, but the second paragraph is all about the classes, which made me think it was kind of redundant.

Also I don't think I would use the phrase "good base theme", and ...

I'm also not thrilled about saying these are Drupal's one "standard set of classes", since the Stable theme doesn't have them, so apparently really the standard is not to have classes.

Also, the headers seem kind of silly to me. The first one is About Classy -- duh. This is a README in the Classy theme, of course it's about Classy. The second one is "More about Classy" but it's just a link to the generic theme guide, not about Classy itself.

So I don't think we need the headers. We could just at the last line say "For more information about theming, see ...".

So.

Can we take another run at this?

About Classy / About Drupal theming would be fine headers. About Classy / More About Classy -- not so much. ;)

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+Drupal 8.

Let's not say "in Drupal 8". Just leave it out.

The reason is that we try to avoid putting explicit "Drupal" or version numbers in our docs in general; version numbers are also problematic because those would need to be changed for new versions.

Other than that, I think this version is pretty good, thanks!

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+Classy is the base theme for the Bartik and Seven core themes. Classy can be

Again, please do not include this sentence. Bartik and Seven have nothing to do with Classy's existence. Their use of Classy is mostly a convenience. It is especially problematic having that as an opening sentence because it gives it greater weight, and implies a primary purpose.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+

I believe the blank line before the code line should be removed. We did that in the Stable README file, so I assume it is spec?

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,15 @@
+Classy is a base theme that can be useful to themers who want a starting point

Replace with "who want to start with custom theme"

Adding Drupal 8 theming guide link to README.txt file.

+
+ABOUT DRUPAL THEMING
+--------------------
+
+For more information, see Drupal.org's theming guide.
+https://www.drupal.org/theme-guide/8

Sorry previously added wrong patch file.

Please see #53

adding @snehi #53 comment

+++ b/core/themes/classy/README.txt
@@ -2,10 +2,10 @@
-Classy is a base theme that can be useful to themers who want a starting point
-for their custom themes without having to add classes throughout templates.
-Themers who want their markup to be as clean as possible (fewer classes) can use
-Stable, core's default base theme.
+Classy is a base theme that can be useful to themers who want to start with
+custom theme without having to add classes throughout templates. Themers who
+want their markup to be as clean as possible (fewer classes) can use Stable,
+core's default base theme.

Sorry but IMO this is a step backwards.

It seemed to me that the phrase "that can be useful to themers who want a starting point for their custom themes" added no value since it just describes what a base theme is. I also wanted to mention that Classy uses a BEM naming syntax.

I have added few lines may be it works:

Please include an interdiff. I suggested that we remove the "that can be useful to themers who want a starting point for their custom themes" sentence in #62. If you feel that sentence should remain in the README, can you please provide a counter-argument to my point.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,20 @@
+Themers who want their markup to be as clean as possible (fewer classes) ¶

Not ok with wording.
Extra trailing space at the end of the line.
Please first suggest and than create a patch, it may be for saving time of yours.
Anyway thanks for contributions and keep it up !!!

Sentence corrected, please review.

+1 for RTBC

Copy and pasting my comment from #65, which was copied from #62: "I suggested that we remove the "that can be useful to themers who want a starting point for their custom themes" sentence in #62. If you feel that sentence should remain in the README, can you please provide a counter-argument to my point."

The paragraphs seem to be all crammed together and there's weird capitalization and such (more on that below), so this still needs more work.

Normally I would really encourage lots of participation in issues but at least in this case it seems that because we have so many different people uploading patches it's delaying the issue, not expediting it. Do we really need 4 different people from the same company working on patches for this issue? I don't think so.

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +ABOUT Classy
   

   Why is Classy not in all caps?

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +The Classy theme is helpful for Themers to start with the custom themes
   

   Why is Themers capitalized?

I'm with @jordanpagewhite, I don't think that sentence is adding much and with the later patches the grammar is not very good either.

Following text removed:
"The Classy theme is helpful for Themers to start with the custom themes
without having to add classes throughout the templates."
and the errors have been corrected.
Please suggest if any more changes required.

sorry, wrong file uploaded. Again uploading the patch file and interdiff.

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,17 @@
   +A theming guide on using Classy can be found at the following URL:
   

   Why t of theming is small ?

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,17 @@
   +see the Theming Guide: https://www.drupal.org/theme-guide
   

   Why T is capital of theming

t will be small in word "theming". Corrected and uploaded patch.

As mentioned in community docs, please provide inter-diff every time whenever you create patch from older patch.
Thanks

Attached interdiff file, please review.

Attached interdiff file, please review.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+A theming guide on using Classy can be found at the following URL:

I would replace "A theming guide" with "Documentation". Particularly, because it is mentioned twice in the same paragraph.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+see the theming Guide: https://www.drupal.org/theme-guide

This change is not correct. We would not have a lowercase "t" used with an uppercase "G". If "Theming Guide" is capitalized, it is because it is the title of the page. If not, then they would both be lowercase. In this case, it looks to be the title, so I would have them both uppercase. "Theming Guide".

The paragraphs are still together. If they're not separate paragraphs, don't wrap early. If they are, add a blank line in between the paragraphs.

patch submitted

readme file to classy

In addition to the previous comments...

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+Classy is a base theme that adds classes using the BEM naming syntax.

What is the BEM naming syntax? Please don't use acronyms in documentation except really really common things like HTML. I have no idea what this is. The acronym needs to be written out, and in addition we probably need to have a reference on what it means (like Wikipedia page or drupal.org documentation page).

RE #83 I would remove the BEM bit altogether. BEM is a naming convention for CSS class names (https://css-tricks.com/bem-101/) but I don't know that I'd say it is strictly implemented in Classy. Besides, it is more of an all around core initiative, not something specific to Classy, so it doesn't add anything special to Classy's description/purpose.

new readme

#79 to #84 has been incorporated into the attached patch. Please review.

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +It uses methodology named *Block,Element,Modifier*(commonly referred to as [BEM](https://css-tricks.com/bem-101/)) as naming convention for classes in HTML and CSS.
   

   I don't think this is the correct way to do this and more than 80 characters in a line

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +For clean markup(fewer classes), *Stable* can be used as core's default base theme.
   

   more than 80 characters in a line

3. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +To use *Classy* as your base theme, set the *base theme* in your theme's *.info.yml* file to *classy*:
   

   more than 80 characters in a line

4. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +	base theme: classy
   

   Empty spaces here

5. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,19 @@
   +To learn how to build your own custom theme and override Drupal's default code, see theming Guide:
   

   more than 80 characters in a line

regarding #86

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,20 @@
   +Classy is a base theme that adds classes using the BEM (a naming convention for
   

   Please spell out the full name and i don't think classy is strictly following BEM.
   Let someone else may be @Jhodgdon can review it better.

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,20 @@
   +.info.ymlfile to "classy":
   

   spcae is missing here.

Thanks for your contribution

See review in #84 please. We do not even want the part about BEM at all. Thanks!

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,20 @@
+.info.ymlfile to "classy":

space needed between .info.yml and file

changes updated with new patch.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,20 @@
+To learn how to build your own custom theme and override Drupal's ¶

Blank space here

1. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,20 @@
   +the markup that help anotate and describe markup elements that render
   

   s/anotate/annotate/

2. +++ b/core/themes/classy/README.txt
   @@ -0,0 +1,20 @@
   +For clean markup (fewer classes), Stable can be used as core's default base
   +theme. To use Classy as your base theme, set the 'base theme' in your theme's
   +.info.yml file to "classy":
   +base theme: classy
   

   Hmm not sure about how these two sentences are together like this. I would consider either splitting this into two paragraphs, with the Stable part after the base theme part, or move the Stable part to the end of the previous paragraph. Also the wording "Stable can be used as core's default base theme" could be improved IMO. To me it's not really the right message, I think it's the "as" that is tripping me up.

Patch attached with those changes and some other minor tweaks more in line with Stable's README. Re-wrapped everything to 80 chars as well.

I'm still not quite happy with the second sentence but don't have any great ideas at the moment on improving it.

Its purpose is to provide many classes throughout the markup that help annotate and describe markup elements that render on the page.

Agreed that the first paragraph is still not great. How about something like this for the first two sentences:

Classy is a base theme that provides many classes in its markup.

That seems like it is much more concise?

And can we also have the last two paragraphs be more parallel? The first one says something like "Documentation for [] is at the following URL: [url]" and the second says "For [] see [title]: [url]".

What we normally do in READMEs is something like this:

See [url] for more information on [whatever it is].

Thanks!

Thanks @jhodgdon!

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+See https://www.drupal.org/theme-guide/8/classy for more information on using
+the Classy theme.

May be it would be make more sense if we can put it below the "About Classy" section.

+++ b/core/themes/classy/README.txt
@@ -0,0 +1,17 @@
+See https://www.drupal.org/theme-guide/8/classy for more information on using
+the Classy theme.

Yes, I agree that this line would probably be better in the About Classy section.

Otherwise, I think this README is looking good!

@snehi Tried reaching you on IRC but couldn't.
Made the changes as per #106.

Empty patch attached earlier. Sorry about that!

@AjitS Please work on assigned issues.

@Snehi I have looked so many issues where you are struggling with the others regarding assigned or unassigned issues. :D :D
Please start work on the issue at the time when you assign it to yourself.

@AjitS : Please check this https://www.drupal.org/node/2172049

@snehi As mentioned earlier, I actually tried to reach you on IRC to check if you are still working on the issue. And I took the issue up after 7 hours, because I thought the change required was small, and completing which would probably push the issue to RTBC.
My intention was not to offend you by any means. Sorry if you feel that way!

@Manjit.Singh Thanks!

Thanks all! Only 114 comments, and we have I think a good README.

added file classy theme README

@sudhanshug this issue is Already RTBC, why are you attaching patch to it.
If you have any issue please write a comment.
Where is the interdiff ?

@sudhanshug: Thank you for your contribution. Every bit helps.

For future reference, please keep in mind a few points so that your patch can be easily reviewed.

• Always explain what you are changing, especially if the patch is already RTBC. Sometimes changes are obvious, but in this case, it wasn't.
• Always include an interdiff whenever relevant. If there is anything you are changing, include an interdiff. You are helping the reviewer, who in turn will help you by giving constructive feedback.
• If there is something wrong, it is always better to read through previous comments or point out that something is wrong. It is better to avoid drastic changes without feedback.

I compared your patch and the one in #110. I see you added points on BEM and changed a few other things. If you see comments #83 onwards, we removed BEM for a reason. If you have a doubt, please ask before making a change.

I've committed the patch in #110 since that is the patch that @jhodgdon (the docs committer) rtbc'd. @sudhanshug if you feel that that documentation can be improved please file a followup issue I made an interdiff of #110 to #116 and it looks like an old version was uploaded.

Committed 791234d and pushed to 8.0.x and 8.1.x. Thanks!