Fix spelling for 29 migrate translation related words

Rerolled

No longer applies

Rerolled

I started to review this.

I compared the word list in the patch to the the word list in the issue summary and they don't match. I applied the patch and searched for non-migration related files that are changed. They are 9

     1     core/lib/Drupal/Core/Menu/MenuTreeStorage.php
     2     core/misc/cspell/dictionary.txt
     3     core/modules/book/src/BookManagerInterface.php
     4     core/modules/book/src/BookManager.php
     5     core/modules/locale/tests/src/Functional/LocaleImportFunctionalTest.php
     6     core/modules/menu_ui/tests/src/Functional/MenuUiTest.php
     7     core/modules/node/tests/src/Functional/Views/NodeFieldTokensTest.php
     8     core/modules/node/tests/src/Kernel/NodeTokenReplaceTest.php
     9     core/modules/system/tests/src/Functional/Menu/BreadcrumbTest.php
    10     core/tests/Drupal/KernelTests/Core/Menu/MenuTreeStorageTest.php

I then looked at those changes, with the following comments.

1. mlid, plid are a Drupalisms used in D8+ and not specific to migrate so we should remove that from the list.
2. chien chiens is used in a Translation test, so that should be removed from the list.
3. tnid is used in #7 and #8. Is that old code from D7?

Removed mlid and plid from IS
Removed chien chiens
And yes tnid seems to be specific to D7

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

#46 was the wrong patch.

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.

error: patch failed: core/misc/cspell/dictionary.txt:974
error: core/misc/cspell/dictionary.txt: patch does not apply

Was an issue with word "vous"

• +++ b/core/lib/Drupal/Core/Menu/MenuTreeStorage.php
  @@ -12,6 +12,8 @@
  +// cspell:ignore mlid
  

  From #45 mlid is a Drupalism. We should keep the word in the dictionary. And that means removing all the ignore lines. :-(

• +++ b/core/misc/cspell/dictionary.txt
  @@ -885,7 +863,6 @@ pjpeg
  -plid
  

  Same here.

• +++ b/core/misc/cspell/dictionary.txt
  @@ -1265,7 +1240,6 @@ titlealert
  -tnid
  
  +++ b/core/modules/node/tests/src/Functional/Views/NodeFieldTokensTest.php
  @@ -43,7 +43,6 @@ public function testViewsTokenReplacement() {
  -      'tnid' => 0,
  
  +++ b/core/modules/node/tests/src/Kernel/NodeTokenReplaceTest.php
  @@ -8,6 +8,8 @@
  +// cspell:ignore tnid
  

  The first test is removing tnid and the second is adding an ignore line. They should do the same thing. What needs to happen is to remove the lines with 'tnid' from the test. I am almost tempted to say that changing these two tests should be in a separate issue. However, I think it is pretty easy to show that tnid is not used in D8+.

  +++ b/core/misc/cspell/dictionary.txt
  @@ -183,8 +181,6 @@ checkboxified
  -chien
  -chiens
  

  Per #45 these are used in a non migration translation test and should not be changed.

  Sorry that my comment in #45 was not clear.

Attempted to address #51

+++ b/core/modules/content_translation/migrations/d6_menu_links_translation.yml
@@ -1,3 +1,4 @@
+# cspell:ignore mlid

If we're keeping mlid in the dictionary we do not need any ignore lines for it. Same with plid.

Not sure what we should do about tnid - this was a Drupalism in D6/D7 ("translated node ID") but it is not used in D8+.

Addressed #54

I should just be reviewing this but I have made changes for plid.

There were still ignore lines for plid so I have removed those. That means that mlid and plid are in the dictionary because there are still instances in core that are not migrate that are using those. That leaves tnid, which has two instances in core outside of migrate and those two are being removed because tnid is not used in Drupal 8+. Those two seems to be left over when moving to Drupal 8.

Removed
-chien
-chiens
From the issue summary.

Searching the rest I see they have been removed successfully from the dictionary.txt file.

1. +++ b/core/misc/cspell/dictionary.txt
   @@ -980,7 +958,6 @@ rebuilder
   -redirections
   

   I know I mentioned this before but I'm not convinced that "redirections" should be disallowed; we use "redirection" as a noun so pluralising it is OK?

2. +++ b/core/modules/field/src/Plugin/migrate/source/d7/FieldInstance.php
   @@ -5,6 +5,7 @@
   +// cspell:ignore localizable
    /**
   

   Missing blank line between cspell comment and docblock.

3. +++ b/core/modules/migrate/tests/src/Kernel/Plugin/MigrationProvidersExistTest.php
   @@ -7,8 +7,8 @@
   +// cspell:ignore entityreference filefield imagefield nodereference
   +// cspell:ignore optionwidgets userreference
    /**
   

   Same.

I would have fixed the latter two on commit but I'm still not sure about the first.

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.

Addressed 2 and 3 points from #58 and raised an MR with the changes.

Moving back to NW as Point 1 from #58 still needs to addressed.

I came here to get this back to NR.
I see it has been converted to an MR, which is not necessary here. This was already RTBC and only minor changes are needed. I also see that there is no diff or anything to show that the MR is correct. Therefor I made an diff between the MR and the patch in #56 and found it was introducing an error. It was adding tsid back to the dictionary.

I am now rerolling this based on #56. This is a re-roll only.

Now to address #58 and #29.

1. Changed to 'redirects'
2,3 Done.

Thanks @quietone for cleaning that up.

Rerolled and re-built the dictionary. When rebuilding the dictionary it added 'please', which we don't want. So, I have removed that from the dictionary in this patch. A followup is needed to investigate the 'please' in COPYRIGHT.txt.

Followup, 📌 Remove remaining uses of string 'bartik' and 'seven' when referring to the removed themes Needs work

Simple reroll. The only changes were to dictionary.txt, which I rebuild as well.

Reroll looks good. Hopefully can commit this one soon.

Thanks @quietone and @smustgrave! Committed 418ce00 and pushed to 11.x. Thanks!

lauriii → closed merge request !4305

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