Discourage use of Stark as a base theme (and possibly mark it internal or hidden)

yes pleaseremove stark, its only D7 ppl that understands what stark is and as i recall there was pushback on this originally cause ppl were still unsure that there was only classy or stable, and didnt really got the concept that stark is no more.

This patch removes stark from the appearance page and also modifies the description telling stable or classy as better option

here the patch changes the help text

Thanks @Meenakshi Gupta.

This text could use some improvements for a few reasons:

1. It's quite long. We should try to make the text as short as possible while still clearly conveying necessary information.
2. The phrase "Intended as" is unnecessary. It might be better to start off with something like:
   

   A testing theme for Drupal's default HTML and CSS.

3. It's removing useful link references that it has HEAD. The links provide valuable information. It would be better to correct the syntax and retain the helpful link.
4. The word "never" is quite strong.
5. It might be better to go the route of actually hiding the theme than to add so many words telling the user not to use it. In order to hide the theme from the UI, we would need to change the assumptions in tests that depend on it, so I can see having that as a separate scope from improving the help text.
6. However, compare stark.info.yml with bartik.info.yml. In that theme, the fact that it is internal is documented in the code comments, rather than in user-facing text. I did not find an existing issue to do that, so we could do it here.

Also a note on core process: In the future, you should not set your own patch to "Reviewed and Tested By the Community". Drupal core uses a peer review process, so the patch should only be set to RTBC by someone else who has read and thoroughly reviewed your patch. Reference: https://www.drupal.org/patch/review

Tagging for subsystem maintainer review, as the decision as to what exactly to do with Stark should have input from the subsystem maintainer.

Oh, you might also find this reference for escaping quotes in YAML helpful: http://yaml.org/spec/current.html#id2534365

@xjm Thanks for correcting me . I have tried to improve the text as much as possible as suggested by you and also hided the theme from the appearance page .
is it right now ?

other changes done but unable to hide stark from appearance page

Added the hidden value to Meenakshi Gupta changes

Patch #30 locks good, hide Stable theme and comment the use of Stark theme. However is needed solve test problems.

@NonProfit please don't queue all the tests. Running tests on drupal,.org is expensive and a change like this doesn't need to be tested on all supported php/db versions. This is a simple string change and that should not be different over different infra versions.

The hidden:true is removed from the latest patches (in #30 I think), that change should still be in the final patch.

Thanks for the clarification. Am I correct in understanding that in a simple change like this, any one of the tests would have been sufficient?

Yes, you don't need to queue any test yourself. Just setting this issue to needs review will automatically trigger the testbot in the default configuration for the latest uploaded .patch/.diff file.

Hidden was not included in the patch as my intent was to continue to expose the Stark to /admin/appearance and allow it to be selected through the UI. With clearer help text, isn't it preferable to allow it to be quickly enabled for testing?

I'm not sure, in theory the stark maintainer should pick this up but @xjm already marked it as such in august 2017. I think it should be hidden but my opinion is just as strong as yours.

Hi ,
I tried patch 40. I haven't been able to apply the patch.

>git apply stark_help-2854576-40.patch
error: patch failed: core/themes/stark/stark.info.yml:1
error: core/themes/stark/stark.info.yml: patch does not apply

I tried different folder just in case and two drupal installation but it didn't apply.
I tried the code manually, it is working well.
So I am not sure if they are a problem with the patch or if I miss something but the code is fine.

Hi NonProfit,

thanks for your answer.
First I a have a question : Are they difference between banch 8.8.x from drupal core clone from git and the zip named Drupal core 8.8.x-dev found on this page ?

I tried your process and your patch applied successfully as you said.

1- But the hidden parameter don't appear in stark.info.yml file after application.
I found that they are a space in front of the + so I change that in the new patch.

2- This is the process that I use to test patch :

• 1) Create a new folder and git clone --branch 8.8.x https://git.drupalcode.org/project/drupal.git
• 2) Change directory to the root cd drupal
• 3) Install vendor and add drush via composer : composer require drush/drush
• 4) Install drupal via drush : vendor/bin/drush si --db-url="mysql://root:@localhost:3306/drupalcontrib" -y
• 3) Add the patch to the root folder
• 3) Apply the patch git apply -v stark_help-2854576-40.patch

With this process I am ready to test the patch before and after and check the code and the UI.
when I did that your patch didn't applied and I don't really understand why.

So I make a new patch based on your code who was great and I test to apply it with your process and mine. In both case it works.

After investigation, I discovered my patch failed due to automated tests who try to install stark theme. But when the patch applied the hidden property all tests linked to stark theme installation failed. So I remove all stark installation in tests.

Locally all the tests pass, lets see if it is the same with the drupal.org bot.

Re-roll in 9.1 and fixed tests, Please review.

patch re-roll in 9.2.

patch re-roll in 9.2,

Patch re-roll for 9.2.
Kindly ignore #53.
I just realised that I had the wrong patch uploaded in #53, apologies for the noise.

Fixing tests.

This is going to need a re-roll to remove mention of seven and bartik, etc. Looks pretty simple for a novice task. And then would be great to have it merged.

Reroll the patch for the D10, as seven and bartik are no longer be there at D10 so i would not be there and make the Claro as default, Updating the patch.

Fixed CCF on #63.

Unsure what that second failing test is caused by, hopefully fixing the first one resolves it.

Oops, missed updating comment.