Comments

kjay created an issue. See original summary.

kjay’s picture

Status: Needs work » Needs review
Issue tags: +dcruhr18
StatusFileSize
new122.46 KB
new1.81 MB

Attached is the patch for the updated recipe including recipe image. This patch adds the new recipe and removes the previous version.

Patch needs general review and checking against https://www.drupal.org/project/drupal/issues/2940146

To help with review I've also added a screenshot of the patch as applied to a clean install of 8.6.

Status: Needs review » Needs work
kjay’s picture

Status: Needs work » Needs review
StatusFileSize
new123.38 KB
new704 bytes

The tests are hard-coded to check whether the first image and first recipe .html file are accessible. The patch in #1 gets rid of the files the tests are trying to test. This patch fixes that.

kjay’s picture

@markconroy pointed out that I had left the baking tin in cm's and not added inches for US English. This has been updated to read "Use a little of the sunflower oil to grease an 8 inch/20cm square baking tin (or similar size)"

eli-t’s picture

Is there anywhere else we use inches and cm togther? I thought we were currently just using inches.

HAL 9000’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new413.4 KB

Hi, I have applied the patch and worked perfectly. Now I see the better recipe and a much improved image as the attached screenshot shows.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

The only cm in the current text is in the existing brownie recipe. Whereas inch is in the brownie and victoria sponge recipe. Let's only use inches for now/

kjay’s picture

Status: Needs work » Needs review
StatusFileSize
new123.38 KB
new1.15 KB

Attached patch changes to inches only.

cferthorney’s picture

Reroll of number 9 so it applys cleanly.

eli-t’s picture

Status: Needs review » Needs work
Related issues: +#2981351: Write alt text for Umami Baking Mishaps article

modules/demo_umami_content/default_content/articles.csv still refers to chocolate-brownie-umami.jpg, but that file has been removed in the patch.

Note this may be changed in #2981351: Write alt text for Umami Baking Mishaps article.

eli-t’s picture

I also suggest the following amends, based on #2940146: Standardize writing style for Umami in English:

6 tablespoons sunflower oil

=> 6 tbsp sunflower oil

1tsp baking powder

=> 1 tsp baking powder

6 heaped teaspoons cocoa powder

=> 6 heaped tsp cocoa powder
I'd consider not specifying heaped and just having more tsp specified. EG 9 tsp cocoa powder has better brevity. We don't use heaped tsp anywhere else.

3 tablespoons of maple syrup

=> 3 tbsp maple syrup

1/4tsp sea salt

=> 1/4 tsp sea salt

230ml organic soya milk unsweetened

=> 230ml unsweetened organic soya milk
I don't think this one is actually in the standards, but it is odd to put the adjective after the noun.

Melt the remaining chocolate by bringing a few centimeters of water

Swap cm for inches. Maybe downgrade a few to a couple

Vidushi Mehta’s picture

Status: Needs work » Needs review
StatusFileSize
new123.37 KB

Here's a patch with the above mentioned changes by #12.

Status: Needs review » Needs work

The last submitted patch, 13: 2953995-13.patch, failed testing. View results

eli-t’s picture

Thanks @Vidushi Mehta! That addresses everything from #12.

We still have to address the fact that the chocolate-brownie-umami.jpg image was removed here but is still referenced by an article. I'd suggest restoring the image file and referring line in /core/profiles/demo_umami/modules/demo_umami_content/default_content/LICENCE.txt so that this issue doesn't break that article.

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

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

Vidushi Mehta’s picture

@Eli-T not getting it can you please explain what needs to be done?

steveparks’s picture

Hi @vidushi-mehta

Reviewing the earlier issues, what @Eli-T was referring to is:

The patch in #2 (and the updated #4) removed the image chocolate-brownie-umami.jpg.

That causes a problem, because the file modules/demo_umami_content/default_content/articles.csv still references that image.

Therefore, please can you update your patch to:

1) reverse this removal - so that the image chocolate-brownie-umami.jpg is still included in Umami

2) reverse the removal of the image credit in /core/profiles/demo_umami/modules/demo_umami_content/default_content/LICENCE.txt

So an updated patch from you would simply be removing these two changes that were included in the patch at #4.

Does that help
Steve

eli-t’s picture

rachel_norfolk’s picture

I'm working on this at the Global Contribution Weekend in Leeds, UK...

eli-t’s picture

rachel_norfolk’s picture

kjay’s picture

StatusFileSize
new107.4 KB

And here is the updated main image for this recipe. This image is already sized and compressed in keeping with the other recipe images.

rachel_norfolk’s picture

heh - seems patch needs re-rolling first. Oh - and my laptop wouldn't charge... 😂

rachel_norfolk’s picture

kjay - the references to the image are to vegan-chocolate-nut-brownies.jpg - I'm assuming you want me to change your uploaded file filename at #23 to match?

kjay’s picture

@rachel_norfolk, please do :)

rachel_norfolk’s picture

Assigned: kjay » Unassigned
Status: Needs work » Needs review
StatusFileSize
new116.27 KB

Okay - here is a patch, re-rolling #13 and incorporating the feedback in #18 and the new image in #23. re-rolling was "interesting" for some unexplainable (to me) reason.

eli-t’s picture

Status: Needs review » Needs work

We need to add vegan-chocolate-nut-brownies.jpg to the LICENCE.txt

We are also inconsistent with respect to hyphenization on "ice-cream" in the Summary, but "ice cream" in the Recipe instruction. Wikipedia and I recommend the latter.

Once these are addressed, I think this is good to go.

starshaped’s picture

Status: Needs work » Needs review
StatusFileSize
new116.98 KB
new4.63 KB

Made updates as noted in #28.

markconroy’s picture

Status: Needs review » Reviewed & tested by the community

Image added to the licence

Lots of 'ice cream' and no 'ice-cream'

Looks like we are good to go.

(and this unblocks another issue, so double YAY!)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 80c1533b8b to 8.7.x and 14b787e79a to 8.6.x. Thanks!

As an Umami content change backported to 8.6.x.

diff --git a/core/profiles/demo_umami/modules/demo_umami_content/default_content/recipe_instructions/vegan-chocolate-nut-brownies.html b/core/profiles/demo_umami/modules/demo_umami_content/default_content/recipe_instructions/vegan-chocolate-nut-brownies.html
old mode 100755
new mode 100644
diff --git a/core/profiles/demo_umami/modules/demo_umami_content/default_content/recipes.csv b/core/profiles/demo_umami/modules/demo_umami_content/default_content/recipes.csv
old mode 100755
new mode 100644

Fixed file modes on commit. Files need to be 644 not 755.

  • alexpott committed 80c1533 on 8.7.x
    Issue #2953995 by kjay, starshaped, rachel_norfolk, Vidushi Mehta,...

  • alexpott committed 14b787e on 8.6.x
    Issue #2953995 by kjay, starshaped, rachel_norfolk, Vidushi Mehta,...

Status: Fixed » Closed (fixed)

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