Fix spelling of words only misspelled in tests, variable name, part 5

Add 'attrvals'.

Updated IS with additional words contained in the MR.

Lesson learned from the last ticket taking some of the words and doing a google search. gethttpclient is a Java function, know that doesn't matter here just thought I'd mention haha.

Reviewing the replacement for the removed words they good.

Shame to see Arrakis go haha

Saving credit for @quietone for doing all the work.

Posted some comments on the MR. The changes to Media don't fit the issue scope defined here or illustrated in the other renamed test variables. I'd move them to a followup entitled "Remove awesome literature reference proper nouns from the CSpell dictionary" if you really must. 😛

All the changes make sense. I did identify a couple places where we're updating one part of the variable name or one of several variable names to use full words, but not the rest, which decreases code readability and consistency a bit. So, I think fixing those suggestions is in scope.

Something I did not do yet is look up each term in the codebase to confirm all the usages and surrounding code are updated correctly without other regressions. I'll do that once the fixes I suggested are addressed one way or the other.

I removed arrakis from the changes here as it is out of scope. I am not making the suggested followup because I want to keep focus on the easier fixes and save what needs discussion for later when the dictionary is smaller, or we have a Drupal dictionary to allow some leniency.

Test failures are in Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest.

Failures are unrelated, in Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest.

Went to RTBC but think something made it in that makes this unmergable. Least according to the MR

It is usually happens that when the dictionary changes all other spelling MR need to rebased.

Rebased and rebuilt the dictionary.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I check the log and the failures are know random failures.

Going to mark but there was another cspell issue I just RTBC'd so imagine whichever lands first the other will need to be updated.

The dictionary removal of arrakis was removed, but not the actual changes to the media test that were out of scope. So I checked out the version of the file from 11.x and pushed that.

This point was missed, or partially missed:

Seems like $attr and $attrs also should be fixed here? Edit: Somehow they are not in the dictionary? 🤔 I would still make the local variable and key names consistently spell out $attribute just for code clarity.

Posted an additional remark on the MR about that, plus one more that I thought I'd posted before but apparently did not regarding the reflected HTTP client variable which is sort of borderline scope-wise. Thanks!

I also see that xjm had to restore changes to MEdiaLibraryTest but I recall making those changes. So, not only did I not publish my comments I lost changes as well. Grrr!

Strange. I do not know how I got assigned in the MR. Removing that. Setting back to NR so reviewers can catch up on my delayed comments.

Seems the threads have been answered. From what I can tell the bulk is about making better variable names and as mentioned that could be it's own meta, not sure how to find other places though.

Add attrs_or_bool to to IS.

Remarking.

Adding attribution for today.

Seriously what power do I have to propitiate to get JS test job 2 to pass... Three requeues so far.

The randomness of night watch and JavaScript tests has been through the roof this week

OK finally.

I went back over and extended the context around each hunk to ensure that all the variable names were correct and consistent within the broader contexts of their tests, and that there were no more name mismatches. And... there's a merge conflict in the dictionary again. zzzt.

Amending credits for #10 which was also helpful.

Thankfully it at least rebased cleanly.

Committed to 11.x and 10.2.x. I also tried to cherry-pick it to 10.1.x, half-expecting what happened, which is that it couldn't be due to the much earlier divergence in the dictionary. So, we'll leave it against 10.2.x like the other related issues. Thanks!

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

quietone → closed merge request !4940