Problem/Motivation

(From #1659682: Consider renaming randomString() and/or adding additional, context-appropriate helpers for random string generation; see also #1672764: Improve documentation of randomString() and randomName())

  • DrupalTestBase::randomName() and UnitTestCase::randomName() does not entirely clarify the function's purpose: to generate a random machine name, for use generating already-validated machine names.

Proposed resolution

  • Rename randomName() to randomMachineName().

Remaining tasks

API changes

  • DrupalTestBase::randomName() is renamed to randomMachineName().
  • UnitTestCase::randomName() is renamed to randomMachineName().
CommentFileSizeAuthor
#19 1676910-randomMachineName-19.patch673.26 KBpenyaskito
#17 drupal-rename_randomName-1676910-17.patch674.71 KBl0ke
#14 drupal-rename_randomName-1676910-14.patch675.77 KBcs_shadow
#10 drupal-rename_randomName-1676910-10.patch646.55 KBcs_shadow
#8 drupal-rename_randomName-1676910-8.patch627.55 KBcs_shadow
#6 drupal-rename_randomName-1676910-6.patch627.54 KBcs_shadow

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Component: documentation » simpletest.module

I don't think this is quite a documentation issue. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes

Updated issue summary.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue summary: View changes
Issue tags: +d8dx, +API change

Is it too late for this to land on D8?

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue tags: -d8dx +DX (Developer Experience), +DrupalWTF
cs_shadow’s picture

cs_shadow commented

Apart from DrupalTestBase::randomName(), there's also a UnitTestCase::randomName(). Does this issue implies to change both or just the first one (that's if its not too late). Someone please clarify.

dawehner’s picture

dawehner
Primary language German
commented

Apart from DrupalTestBase::randomName(), there's also a UnitTestCase::randomName(). Does this issue implies to change both or just the first one (that's if its not too late). Someone please clarify.

You are 100% right, we should sync those 2 code snippets.

cs_shadow’s picture

cs_shadow commented
Status: Active » Needs review
StatusFileSize
new drupal-rename_randomName-1676910-6.patch627.54 KB

Attaching a patch where I replaced every occurrence of randomName to randomMachineName. Hope it doesn't breaks anything.

Status: Needs review » Needs work

The last submitted patch, 6: drupal-rename_randomName-1676910-6.patch, failed testing.

cs_shadow’s picture

cs_shadow commented
Status: Needs work » Needs review
StatusFileSize
new drupal-rename_randomName-1676910-8.patch627.55 KB

Sorry, patch in #6 was not against latest HEAD.

Status: Needs review » Needs work

The last submitted patch, 8: drupal-rename_randomName-1676910-8.patch, failed testing.

cs_shadow’s picture

cs_shadow commented
StatusFileSize
new drupal-rename_randomName-1676910-10.patch646.55 KB

This one should work.

cs_shadow’s picture

cs_shadow commented
Status: Needs work » Needs review
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Doesn't apply anymore.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue tags: +Novice
cs_shadow’s picture

cs_shadow commented
Status: Needs work » Needs review
StatusFileSize
new drupal-rename_randomName-1676910-14.patch675.77 KB

Quick reroll.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue tags: -Needs reroll +Needs change record

I guess we will need a change record draft.

Status: Needs review » Needs work

The last submitted patch, 14: drupal-rename_randomName-1676910-14.patch, failed testing.

l0ke’s picture

l0ke
Primary language English
Location Ukraine
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-rename_randomName-1676910-17.patch674.71 KB

Reroll against latest HEAD.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Does not apply anymore.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 1676910-randomMachineName-19.patch673.26 KB

Used rename with phpstorm.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue summary: View changes
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Created draft change notice: https://www.drupal.org/node/2315865

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue summary: View changes
cs_shadow’s picture

cs_shadow commented
Status: Needs review » Reviewed & tested by the community

Tweaked the Change Recoed a bit.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs change record

Committed 9eac446 and pushed to 8.0.x. Thanks!

  • alexpott committed 9eac446 on 8.0.x 
    Issue #1676910 by cs_shadow, penyaskito, lokeoke | xjm: Rename...
m1r1k’s picture

m1r1k commented
Issue tags: +#ams2014contest

Status: Fixed » Closed (fixed)

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