Add an article to Umami - Oatmeal

Working on this as part of Drupalcon Nashville.

W00t!

lots of questions about what to do.

I proofed the document, and made some very minor changes. (see attached). Can you diff to see what's updated?

What now?

@kjay: You can generate a link based on an issue number using the syntax [ #2960483] (without the extra space). For example:

• #2960483: Add an article to Umami - Mocktails
• #2939908: Add an article to Umami - Dairy-free chocolate

Added patch for the same. Please review.

I've tested the patch. Although it produces error such as fatal: can't open patch 'add-an-article-to-umami-oatmeal-2960486-10.patch': No such file or directory, it still applies properly. (The error is coming from the included image data.)

There are a couple of problems.

1. The data is not currently imported. This is because a new line needs to be added to core/profiles/demo_umami/modules/demo_umami_content/default_content/articles.csv so the importer knows about the new article.
2. Currently a placeholder image is used. This this needs to be replaced with a properly licensed photo.

The h1 line "Give your oatmeal the ultimate makeover" at the top of the text file duplicates the Title of the node. Also the input filter will remove the h1 tag.

Added patch with image added and made changes in licence.txt. Please review.

Working on this as part of Drupalcamp Scotland.

Per comment #14, the top paragraph of text ("Give your oatmeal the ultimate makeover") still repeats the title, and should be removed.

In the current paragraph 2 I’d recommend a comma after the word well to confirm the meaning:
“Well, before you write off that boring jar of oats, you might want to take inspiration from our topping ideas that will take your oatmeal from bland to creatively delicious in just a few minutes.”

In current paragraph 3, I’d recommend the first sentence to be:
“Call it oatmeal, porridge, hot cereal or just plain oats, it is loved the world over; but recently, the humble oat is getting a funky makeover. “

In current paragraph 4 I’d suggest a comma added to the first sentence:
“It sounds great, doesn’t it?”

Under the heading: “Porridge creme brûlée”, I’d recommend adding a comma to the last paragraph:
“If you are really professional, you can use a blow torch.”

Rerolled, and applied suggestions in #19. No interdiff due to reroll.

We're here at Twin Cities Drupal camp and are going to take a look at this issue.

Applied patch, all changes in #19 were made. This patch adds accents to the "Crème brûlée" subhead that were missing.

Looking into why the last submit failed.

I'm not sure why the previous submit failed. I've re-done the steps that we did which were:

* Apply #21
* Tested that #21 met the changes listed in #19. It did.
* Updated creme brulee to read crème brûlée
* Created patch #28

And now I'm uploading. Hopefully, it will pass tests. If not, hopefully, the folks at the sprint can tell us what we did wrong :)

#28 is going to fail as well. Following the instructions on https://www.drupal.org/node/707484 led us to include uncommitted changes created by lando in our patch. I've re-done the previous steps again:

* Apply #21
* Tested that #21 met the changes listed in #19. It did.
* Updated creme brulee to read crème brûlée
* Created patch #29

And now I'm uploading. I'm feeling optimistic this time.

The order of the commit IDs in the git diff matters. It should be git diff older newer > patch

Reviewing this patch at Design4Drupal in Boston.

The patch applies cleanly and the new article is added to the content. There is a message when applying the patch:

warning: 5 lines add whitespace errors.

All the whitespaces errors come from the .jpg binary and there is nothing we can do about binaries.

The content reads correctly to my eyes. Marking this issue as RTBC.

Nice, thanks for working on this! Just a few small questions and nitpicks.

1. So, one thing I notice overall is that this article uses British English (BE) spellings and punctuations, but Drupal core and our content style guide use American English (and there is a British English translation).

   I realize the content author here is a BE speaker and this probably also applies to other Umami content that's already in core. :) But since there's a British English translation of Drupal, I think probably our content should be in American English here too? Worth discussing, at least.

   Some examples of this are:

   • Serial commas: According to Drupal's content style guidelines, there should be a comma before the "and" in a series of things (e.g. "cinnamon, chopped walnuts and a dollop of sweetened cream cheese" should be "cinnamon, chopped walnuts, and a dollop of sweetened cream cheese").
   • Spellings: "savoury", "fibre", etc.

2. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/LICENCE.txt
   @@ -18,6 +18,7 @@ veggie-pasta-bake-umami.jpg by Keith Jay
   +oatmeal-fruit-syrup-topping.jpg
   

   The other source images have credits, but this one does not. Any particular reason?

3. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +It is vegan, gluten free, low in fat, high in fibre and can even lower cholesterol - but oatmeal is boring, right? Well, before you write off that boring jar of oats, you might want to take inspiration from our topping ideas that will take your oatmeal from bland to creatively delicious in just a few minutes.
   

   "Gluten-free" should be hyphenated.

4. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +It sounds great, doesn’t it? So before you close the cupboard door on that bland looking jar of oats, we think you should try some of these scrumptious topping ideas to inspire your very own oatmeal makeover.
   

   "Bland-looking" should be hyphenated.

5. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +So simple - but why has no-one thought of it before? Put your cooked oatmeal into a ramekin, sprinkle with caster sugar and pop under the grill. The sugar will harden, giving you the delight of cracking through the surface and scooping out the creamy deliciousness beneath. If you are really professional, you can use a blow torch.
   

   "No one" should be two words, not hyphenated.

   I didn't know what "caster sugar" was either. Apparently that is another British name. :)

6. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +Cook your oats with grated carrots and a little sugar, then top with a sprinkle of cinnamon, chopped walnuts and a dollop of sweetened cream cheese. You could even claim you are getting one of your five a day! If you don’t love this, there’s something wrong with your tastebuds.
   

   I also wondered if the "five a day" (referencing servings of vegetables, I think) might be a little too culturally and generationally specific. (The "food pyramid" etc. taught in US schools at least has changed in recent decades.)

   Aside: This sounds delicious!

7. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +This low sugar delight will meet all of those chocolate cravings and is the perfect dessert. Add a spoonful of cocoa to your oatmeal as it cooks, then top with a few squares of dark chocolate. As it melts, simply swirl into the bowl as artfully as you can. If you are really decadent, a squirt of whipped cream will add further delights.
   

   "Low-sugar" should be hyphenated.

A screenshot would also be nice for reviewing the article. Thanks!

@xjm thanks for taking a look,

Articles for Umami should absolutely not use British spelling etc. There is a writing style for Umami that encapsulates much of what you said in #35 in https://www.drupal.org/project/drupal/issues/2940146. There could be a better place for that, and as noted in the last OOTB initiative call, we need to make it more prominent on content issues that that exists, so that people that help out by reviewing are aware there are a set of standards to review against.

As most of the team writing articles are from the eastern side of the Atlantic, it is hard to spot terms such as caster sugar that aren't used in the US. We had some confusion with cornflour before!

Also noted:

1. The alt text is just "oatmeal, syrup". This should be improved to be more descriptive, in line with alt text used on other articles and recipes. For example, /articles/the-umami-guide-to-our-favourite-mushrooms has "A delightful selection of mushroom varieties laid out on a simple wooden plate."

2. According to https://www.bbc.com/food/caster_sugar, caster sugar is called "superfine sugar" in the US. Or we could just say "sugar", I doubt it makes that much difference to the recipe.

3. Totally agree on the 5 a day objection - that's a specific UK government drive.

Addressed xjm's items 1-7.
Addressed Eli-T's items 1-3.

Also removed .rej and .orig files mistakenly included back in #21

Screenshot of the article page added.

Reroll, not sure why last patch wouldn't apply, added --binary in diff this time.

Going to take a look at this today at Dev Days...

Review.

From xjm’s comments:

1) still a couple of listing commas missing
2) resolved
3) resolved
4) resolved
5) resolved
6) resolved
7) resolved

From Eli-T’s comments:

1) resolved
2) resolved
3) resolved

So, the only issue still remaining are the highlighted areas in the attached screenshot, where listing commas should be added.

Also, as an aside, if you want to be able to check alt text on an iPad, just turn on VoiceOver! (Can’t believe it took me ten minutes to think of this! 😂)

@rachel_norfolk i have added the patch

Thanks for the patch johnny but it makes changes to other pages, which are outside the scope of this issue. Can you update that patch, please, changing only the three missing commas outlined in #41?

Then I think we will be ready!

Thanks

@rachel i updated the patch please review

Thanks johnny!

Latest patch incorporates all the feedback made so far and corrects the three missing listing commas. Looks RTBC to me!

To get tests to pass I needed to diff with the —binary parameter. Rachel, did the image file get created ok in your test? I’m not near a computer to test but can look later.

Just a re-roll of #45 but with --binary to try to make tests happy.

Also confirmed patch in #45 creates the file just fine (as does this one).

Not sure why tests fail on 'trailing whitespace' within the jpeg but I'm going to mark this back to RTBC for now. Can anyone with more experience with tests on d.o explain?

The image looked fine to me

I've had a quick look at this.

A different commit (#2983996: Dedupe author in Umami) changed articles.csv, which this patch also changes. So the patch in #45/#49 needs to be re-rolled against the current 8.6.x head of core. If @johnny_bs.221B or @tanc can do this, that would be great!

I've added the "Needs reroll" tag.

Reroll added

Back to RTBC, the trailing whitespace thing happens with images a lot, so don't worry about it.

I've installed the patch in #54 against the latest 8.6.x via simplytest.me.

The patch applies, and Umami installs correctly.

The change made in #2983996 that caused the reroll to be required is still present (one Megan Collins, and she's the right one).

I've re-reviewed the article as created during install against all the points made throughout this issue and am happy everything has been addressed.

Therefore RTBC+1!

Thanks everyone :)

+++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/articles.csv
@@ -5,3 +5,4 @@ The umami guide to our favorite mushrooms,the-umami-guide-to-our-favourite-mushr
+Give your oatmeal the ultimate makeover,give-your-oatmeal-the-ultimate-makeover.html,Umami,articles/give-your-oatmeal-the-ultimate-makeover,oatmeal-fruit-syrup-topping.jpg,"Oatmeal topped with a vibrant mix of berries, nuts, and seeds","Vegan,Vegetarian,Oats,Breakfast,Dinner,Desert"

1. "Desert" - must be very dry porridge given that it's tagged with this tag 🏜🌵

2. Wondering how porridge is related to dinner? Not insisting to remove it but just want to make sure it's not there by an accident 😛

Ps. that photo is beautiful! 🤩

hi @lauriii , what exactly needs to be changed.

1. The "desert" tag should be changed to "dessert".
2. I think the "dinner" tag should be removed as well, but not 100% sure about that.

@borisson_ i have made the following changes
1. The "desert" tag should be changed to "dessert". - changed
2. I think the "dinner" tag should be removed as well, but not 100% sure about that. - removed

The patch in #61 appears to be incorrectly generated, and doesn't create the give-your-oatmeal-the-ultimate-makeover.html file on application.

Agree Desert -> Dessert. I feel this mistake has been made before but can't find it right now.

However it's possible the Dinner tag is deliberate as the article briefly mentions savory porridge. Or because dessert could be a component of dinner. Will page @kjay for clarification of intent.

Agree Desert -> Dessert. I feel this mistake has been made before but can't find it right now.

It was, and fixed in #2984001: Typo in default content (recipes)

As @Eli-T mentioned in #63, the patch wasn't generated correctly. This should be done again based on #54 and the changes in #60 need to be applied again.

It looks like #61 inverted interdiff and patch file names. So it's possible that renaming those files will be enough.

@borisson_ do u want me to rename my patch and submit again

Yes, what was the interdiff previously should be the patch file (at least, that's what it looks like :))

Added a full patch with a change.

I've had a look at #69.

The patch applies cleanly against 8.7 and the tags are correct.

The section on "Super seeds" says:

For a more savory meal option, cook your oats with water and a little salt and sprinkle with toasted seeds.

This has two "and"s in quick succession. Should this be looked at and possibly use a serial commas.

Otherwise, this looks like it's nearly ready.

1. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +Call it oatmeal, porridge, hot cereal or just plain oats, it is loved the world over; but recently, the humble oat is getting a funky makeover. Long gone are the days of stodgy breakfast bowls of thick gloop, because oats are getting served up as a delicious meal that can suit any time of the day. Trendy build-your-own oatmeal bars are popping up, and they are providing the ultimate oatmeal experience. Diners can choose their oats to be made with milk or water and then go to town with their choice of favorite toppings - sweet, savory or even spice.
   

   Serial comma needed before the "or" in "Call it oatmeal, porridge, hot cereal or just plain oats"

   "sweet, savory or even spice." – should be "spicy", and there should be another serial comma before the "or".

2. +++ b/core/profiles/demo_umami/modules/demo_umami_content/default_content/article_body/give-your-oatmeal-the-ultimate-makeover.html
   @@ -0,0 +1,43 @@
   +For a more savory meal option, cook your oats with water and a little salt and sprinkle with toasted seeds. You can choose any that you enjoy, but pumpkin seeds, sesame, linseed, and sunflower seeds are especially good. Some supermarkets also sell seed blends to make the job even easier.
   

   After talking with @kjay, the first sentence should be "For a more savory meal option, cook your oats with water, a little salt, and sprinkle with toasted seeds."

Did changes as suggested by @John Cook

few changes were not done so here is the correct patch

Thanks folks!

Could you provide an interdiff please? It makes it much easier to review when you're building on a previous patch: https://www.drupal.org/documentation/git/interdiff

interdiff of 2960486-73.patch and 2960486-69.patch

Screenshots were added in comments #41 and #46

I've tested the patch in #74 and confirm it applies cleanly to 8.6.1 and also against 8.7.x.

I've also checked back on the discussion and confirm that it has applied all changes agreed in this thread so far.

I've checked the suggestion that I made in #71 – all have been applied.

I've also tested the patch from #74 against 8.7.x and it applied cleanly.

As this has already been check by @steveparks, I'm setting this to RTBC.

Committed 089f391 and pushed to 8.7.x. Also cherry-picked 349314c and pushed to 8.6.x. Thanks! 😋🥣