Drupal Core has some pretty atrocious spelling errors in it.

Now, on one hand, we're geeks. Why would spelling matter?

However, as part of reaching a global audience, misspelled English words are a real affront to folks whose first language isn't English.

Finding text through search/replace is also more of a challenge when not all of the words are consistently used.

It also just looks pretty amateur. If this is an enterprise system, we can't be filled with lots of stupid typos in the code.

Javascript - #2383865: Spellchecking Drupal - Javascript
Comments - #2383863: Spellchecking Drupal - Comments
PHP - #2383871: Spellchecking Drupal - PHP

------

Original:

The core is spellchecked. The typos which don't break the code were fixed.

CommentFileSizeAuthor
#101 drupal7-spellchecked-2329703-101.patch218.34 KBmaximpodorov
#99 2488476-fix_14_typos-1.patch8.33 KBPere Orga
#96 drupal8-spellchecked-modules_comments-2329703-96.patch112.89 KBmgifford
#82 drupal8-spellchecked-modules_comments-2329703-82.patch122.12 KBrpayanm
#82 drupal8-spellchecked-modules_comments-2329703-interdiff.txt1.28 KBrpayanm
#78 drupal8-spellchecked-modules_comments-2329703-78.patch121.16 KBrpayanm
#78 drupal8-spellchecked-modules_js-2329703-78.patch3.62 KBrpayanm
#71 spellchecking_drupal-2329703-71.patch120.49 KBValentine94
#69 spellchecking_drupal-2329703-69.patch240.07 KBValentine94
#66 drupal8-spellchecked-modules_comments-2329703-66.patch121.95 KBrpayanm
#65 drupal8-spellchecked-modules_js-2329703-65.patch3.62 KBrpayanm
#64 drupal8-spellchecked-modules_code-2329703-64.patch43.54 KBrpayanm
#58 2329703-58.patch43.54 KBrpayanm
#49 drupal8-spellchecked-modules_code-2329703-49.patch44.95 KBmgifford
#35 drupal8-spellchecked-modules_code-2329703-35.patch45.72 KBmaximpodorov
#33 drupal8-spellchecked-modules_code-2329703-33.patch46.42 KBmaximpodorov
#32 drupal8-spellchecked-modules_comments-2329703-32.patch124.29 KBmaximpodorov
#31 drupal8-spellchecked-modules_js-2329703-31.patch3.85 KBmaximpodorov
#27 drupal8-spellchecked-modules_code-2329703-27.patch51.33 KBmaximpodorov
#26 drupal8-spellchecked-modules_js-2329703-26.patch10.94 KBmaximpodorov
#24 drupal8-spellchecked-modules-2329703-24.patch183.87 KBmaximpodorov
#21 drupal8-spellchecked-modules-2329703-21.patch183.79 KBmaximpodorov
#18 drupal8-spellchecked-lib-2329703-11.patch42.13 KBmaximpodorov
#10 drupal8-spellchecked-themes-2329703-10.patch2.14 KBmaximpodorov
#10 drupal8-spellchecked-tests-2329703-10.patch67.78 KBmaximpodorov
#9 drupal8-spellchecked-misc-2329703-9.patch4.23 KBmaximpodorov
#9 drupal8-spellchecked-includes-2329703-9.patch8.13 KBmaximpodorov
#1 drupal7-spellchecked-2329703-1.patch219.1 KBmaximpodorov
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

maximpodorov’s picture

Status: Active » Needs review
FileSize
219.1 KB

Status: Needs review » Needs work

The last submitted patch, 1: drupal7-spellchecked-2329703-1.patch, failed testing.

Status: Needs work » Needs review
andypost’s picture

Component: other » documentation
Issue tags: +Documentation

please provide a way you does this patch to allow reviewers make sure that nothing is left ;)

maximpodorov’s picture

That was just a magic. :)
I ran code inspection in my IDE (using spell checker only) and fixed typos. After many iterations with adding legitimate words in IDE's dictionary and fixing obvious typos in the core, there are about 1500 unknown words left which are mostly country and language names, Latin words from "Lorem ipsum", and strange php variable names. This amount is quite observable, so I think most problems are fixed by this patch. There are test function names with typos and even Drupal variable name and JavaScript property name with typos, but I don't want to touch them.

jhodgdon’s picture

This is a nice patch probably... but very large and thus hard to review... Also, we have a policy of not fixing bugs in Drupal 7 until the corresponding bug in Drupal 8 has been fixed first. So, this one is going to be difficult to get committed.

maximpodorov’s picture

Can typos in D8 be the corresponding bugs to the typos in D7?

jhodgdon’s picture

Version: 7.x-dev » 8.0.x-dev
Status: Needs review » Needs work
Issue tags: -Documentation +Needs backport to D7

Basically, this issue needs to be Drupal 8, and then backported to Drupal 7. But you will have better luck getting it reviewed if you break it up into smaller patches.

maximpodorov’s picture

Title: Spellchecking Drupal 7 » Spellchecking Drupal
Status: Needs work » Needs review
FileSize
8.13 KB
4.23 KB

Here are patches for Drupal 8 split by the core/* directory.

maximpodorov’s picture

maximpodorov’s picture

  • jhodgdon committed 73c66d4 on 8.0.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    

  • jhodgdon committed 2a287f3 on 8.0.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    

  • jhodgdon committed bedbd0a on 8.0.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    

  • jhodgdon committed c1442c5 on 8.0.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
jhodgdon’s picture

Thanks! Those four patches in #9 and #10 have been committed to Drupal 8.0.x. I suspect you have more patches coming...

By the way... one of the patches had several variable names whose spelling was fixed... I had to check those more carefully, but they were fine. I'll wait to commit the next batch of patches until bot reviews are done because when you change variable names, we need to make sure the tests still pass. That next patch is pretty big!

jhodgdon’s picture

Hiding the 4 patches I committed and the huge one for 7. Something happened to the patch in #11, can you reupload maybe?

maximpodorov’s picture

The huge patch is huge because of diff context. The changes are small.

jhodgdon’s picture

Status: Needs review » Active

Committed the patch in #18 to 8.0.x. Back to "Active" status in case there are more patches coming. Thanks again!

  • jhodgdon committed 9cf1459 on 8.0.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
maximpodorov’s picture

One more patch for 'modules' directory. And that's all for Drupal 8. Only 'assets' and 'vendor' directory are left, but they contain external files not owned by Drupal.

maximpodorov’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 21: drupal8-spellchecked-modules-2329703-21.patch, failed testing.

maximpodorov’s picture

Status: Needs work » Needs review
FileSize
183.87 KB

This patch is the final one.

jhodgdon’s picture

Status: Needs review » Needs work

If you are going to fix the spelling of "htmls", please change it to "HTML". It's an acronym.

Also, I think in BlockContentSaveTest, determing should be determining, not determine.

And I'm a bit concerned about changes to .js files. They are not tested by the bot. So can you put them in a separate patch so we can review/test them manually? Actually... there are quite a lot of code changes in this patch (as opposed to comment changes). Can you please put them in a separate patch? I'm comfortable doing a review-and-commit on the comment spelling changes, but if you are changing PHP method names, or strings in PHP code that are part of tests, they need to be reviewed more carefully. This patch is pretty big...

maximpodorov’s picture

Status: Needs work » Needs review
FileSize
10.94 KB

The patch for modules JS code.

maximpodorov’s picture

FileSize
51.33 KB

This patch is for modules code changes.

maximpodorov’s picture

And the last patch for comments.

jhodgdon’s picture

Oh, sorry, I think I didn't make myself clear.

Any change that is in a code comment, you can put into one patch. That patch I can easily review and commit quickly.

Changes that are to actual JS code (changing function or variable names or literal strings in code) should go in a second patch, so it can be reviewed more carefully. Your patch in #26 has a bunch of comment changes in it -- please add those to the comments-only patch and they'll get taken care of sooner.

Changes that are to actual PHP code (changing function or variable names or literal strings in code) should go in a third patch, so it can be reviewed more carefully. Your patch in #27 has a bunch of comment changes in it -- please add those to the comments-only patch and they'll get taken care of sooner.

The patches in #26 and #27 will take a while to review and since they just fix typos, they are not going to be anyone's highest priority right now... so the more you can get out of those patches and into a "this is just typos in comments" patch, the better.

Status: Needs review » Needs work

The last submitted patch, 27: drupal8-spellchecked-modules_code-2329703-27.patch, failed testing.

maximpodorov’s picture

Status: Needs work » Needs review
FileSize
3.85 KB

JavaScript code change patch.

maximpodorov’s picture

Comments patch.

maximpodorov’s picture

Status: Needs review » Needs work

The last submitted patch, 33: drupal8-spellchecked-modules_code-2329703-33.patch, failed testing.

maximpodorov’s picture

FileSize
45.72 KB

PHP code patch without incorrect code left from the outdated commits.

maximpodorov’s picture

Status: Needs work » Needs review
mbrett5062’s picture

Some small nitpicks/corrections I found in the "Comments" patch. #32

  1. +++ b/core/modules/block/src/Tests/BlockLanguageTest.php
    @@ -11,7 +11,7 @@
    + * Tests if a block can be configure to be only visible on a particular
    

    I think this would read better as so:

    Tests if a block can be configured as only visible on a particular language.

    *NOTE language can now move up, so making this a single line, now less then 80 characters.

  2. +++ b/core/modules/config/src/Tests/ConfigExportImportUITest.php
    @@ -175,7 +175,7 @@ public function testExportImportCollections() {
    +    // Create the tar contains the expected content for the collections.
    

    Should this be:

    // Check the tar contains the expected content for the collections.

  3. +++ b/core/modules/system/src/Tests/Entity/EntityAccessControlHandlerTest.php
    @@ -2,7 +2,7 @@
    + * Contains \Drupal\system\Tests\Entity\EntityAccessHControlHandlerTest.
    

    Extra 'H' left in here I think, should be:

    * Contains \Drupal\system\Tests\Entity\EntityAccessControlHandlerTest.

  4. +++ b/core/modules/views/src/Plugin/views/argument_validator/ArgumentValidatorPluginBase.php
    @@ -102,7 +102,7 @@ public function validateArgument($arg) { return TRUE; }
    +   * Some plugins alter the argument so it uses something else internal.
    

    Should this be "internally" not "internal"

  5. +++ b/core/modules/views/src/Plugin/views/field/MachineName.php
    @@ -12,7 +12,7 @@
    + * Field handler which allows to show machine name content as human name.
    

    I think this reads better as follows:

    * Field handler which shows machine name content as a human readable name.

  6. +++ b/core/modules/views/src/Plugin/views/query/Sql.php
    @@ -151,7 +151,7 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
    +   *   Should the view be distincted.
    

    Should be "distinct" not "distincted"

  7. +++ b/core/modules/views/src/Tests/Handler/FieldWebTest.php
    @@ -284,7 +284,7 @@ public function testAlterUrl() {
    +    // Tests the linkclass setting and see whether it actually exists in the output.
    

    "linkclass" should be "link_class"

  8. +++ b/core/modules/views_ui/css/views_ui.admin.theme.css
    @@ -691,11 +691,11 @@ td.group-title {
    + * Applies a overridden(italics) font style to overridden buckets.
    

    "Applies an" not "Applies a" I think reads better.

mgifford’s picture

Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

I looked through drupal8-spellchecked-modules_code-2329703-35.patch

It's in function & variable names, tests & the documentation. Quite hilarious actually. All those folks whose first language isn't English who who may be stumped by all these errors. All the propagation of these errors (because folks copy/paste prior examples). I'm including a list of errors discovered that were uncovered in the patch:

  • Absense
  • acess
  • assigments
  • avaiable
  • auto-incrament
  • bloack
  • cachable
  • caluse
  • Corect
  • correclty
  • Definintion
  • Deletetions
  • deltion
  • Denormailzation
  • diplayed
  • Enity
  • formater
  • groubpy
  • greather
  • Keysword
  • Lllama
  • messsages
  • occurance
  • optipns
  • overriden
  • Peole
  • posible
  • prefereed
  • proprocess
  • registed
  • renderd
  • Reponse
  • resetted
  • runned
  • submtited
  • succeedded
  • Successfull
  • toolar
  • touples
  • udpate
  • vaildation

Not that I'm much of a speller or noticed any of these on my own. Even when comparing lines it was sometimes hard to see where the problem was.

This isn't a problem with the patch, but noticed that the whole $optipns key isn't being used in this foreach:

    foreach ($display->getComponents() as $name => $optipns) {
      if ($name != $field_name) {
        $display->removeComponent($name);
      }
    }

Is it worth removing in another issue?

mgifford’s picture

Status: Reviewed & tested by the community » Needs work

Just saw @mbrett5062's review.

jhodgdon’s picture

Status: Needs work » Needs review

Regarding mbrett5062's review, this patch is *just* fixing spelling errors, as other patches did above. If it gets into fixing grammatical errors (which existed before this patch), it gets more difficult to review. So let's not fix those here. Or any other issues. Just spelling of individual words.

So there are 3 active patches... which one(s) have you reviewed mgifford? And if it's the code patches, I'm very concerned about the code changes and they need to be reviewed more carefully than just verifying that the spellings are now correct...

mbrett5062’s picture

I think 1,3,6, & 7 from my review should be considered spelling corrections, not grammatical changes.

#NOTE 1 points out that "configure" should be "configured", as well as the grammar change.

I also think that 2 can be considered a spelling correction.

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

@jhodgdon I am marking the code #35 & #31 as RTBC. The corrections to the JS code brings us a few additional errors like:

  • Callack
  • Countextual
  • Commiting

The bot would have most likely discovered any changes in variable names that would cause problems. For the next 6hrs there will be examples running up here, here & here for folks to investigate. They all look good to me.

I agree that the comment code in #32 still needs a bit more work. Some of @mbrett5062's suggestions go beyond spell checking the comments. I think they should be made though if @jhodgdon thinks they are an improvement. I don't know that they need to be brought into a new issue but maybe.

Maybe the JS, Code & Comments should be 3 separate issues so we can track them separately and not get confused. That being said, these seem pretty simple. It is important though that they are fixed before they spread.

These are pretty straight forward changes though...

star-szr’s picture

With any patch like this, it's worth bringing up the awesomeness that is git diff --color-words. Hat tip to @xjm for that one.

mgifford’s picture

Nice. That's way easier. Thanks @Cottser & @xjm. Produces a display like:
$this->assertFalse($javascript['core/misc/collapse.js']['preprocess'], 'Setting cache to FALSE sets proprocesspreprocess to FALSE when adding JavaScript.');

So you don't have to dig though and try to find the differences in the diff (like I did).

maximpodorov’s picture

Maybe it's better to wait for #31 and #35 committing, and then continue to fix #32 in this issue.

mgifford’s picture

Part of my urgency on this is that in my experience with other low hanging issues like:
https://www.drupal.org/project/issues/search?issue_tags=typography

They are big patches that need to be rerolled & rerolled & rerolled... Never really getting into Core.

Perfect is the enemy of good. This is an issue that really doesn't need to be perfect. It's clearly an improvement that will address some embarrassments.

jhodgdon’s picture

There is another issue marked "avoid commit conflicts" that also touches a bunch of files in core/modules. I am not going to take action on any of these patches until #2331919: Implement StackNegotiation is finished (that is higher priority). Sorry.

Also I'm not going to commit the JS and PHP code changes patches. Too risky for me. So... I plan to review/commit the "comments" patch when there aren't any possibly-conflicting "avoid commit conflicts" issues. Then we'll need to change the component to something other than "documentation" and get another committer involved.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 35: drupal8-spellchecked-modules_code-2329703-35.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
44.95 KB

Looks like testTagDeletetions was corrected. Not before Google caught it in the API mind you https://www.google.ca/search?q=testTagDeletetions

Anyways, there are over 40 spelling mistakes which are caught in the modules patch which I just re-rolled.

jhodgdon’s picture

Since this issue has multiple patches, please hide old files when you upload replacement patches. Thanks!

mgifford’s picture

That's a good pattern. I haven't applied that in the past... Good suggestion though. Thanks for implementing it.

tim.plunkett’s picture

Priority: Major » Normal

Sorry @mgifford, this is not a major bug by any definition.

mgifford’s picture

Fine... But then we're bound to fix this problem in 20 separate issues that get opened over the next year rather than this one were it's been consolidated.

jhodgdon’s picture

@mgifford - tim.plunkett is right. Fixing typos like this -- especially ones that end users do not see and that do not affect functionality -- does not take precedence over substantive issues. We will get this done; if you look at the history above you'll see that I've already committed quite a number of patches on this issue.

The last 3 patches that have been submitted here are just especially problematic to commit, as they are large (therefore can lead to rerolls for many many other more substantive issues) and the two that affect code need to be examined very carefully before they can be committed. Patience, please -- these last few patches have only been here for a few days. There are plenty of more important issues in the RTBC list that have been waiting at least as long, most of the time, before they're finally committed.

maximpodorov’s picture

Maybe Drupal 7 can be cleaned from the typos meanwhile?

jhodgdon’s picture

Our policy is always to fix Drupal 8, then port to Drupal 7.

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
rpayanm’s picture

Status: Needs work » Needs review
FileSize
43.54 KB

rerolling...

mgifford’s picture

Issue tags: -Needs reroll
mgifford’s picture

Issue tags: +Needs reroll

error: patch failed: core/modules/language/src/Tests/LanguageUILanguageNegotiationTest.php:314
error: core/modules/language/src/Tests/LanguageUILanguageNegotiationTest.php: patch does not apply

mgifford queued 58: 2329703-58.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 58: 2329703-58.patch, failed testing.

jhodgdon’s picture

There are several patches now on this issue.

Can you please:

a) Name files consistently so we know which reroll applies to which patch without having to check them all.
b) Hide obsolete patch files so only patches currently under consideration are still visible at the top of this issue.

rpayanm’s picture

Status: Needs work » Needs review
FileSize
43.54 KB

ohh sorry I never worked with a one issue with multiples patches :)

reroll again...

rpayanm’s picture

FileSize
3.62 KB

js

rpayanm’s picture

rpayanm’s picture

jhodgdon’s picture

Thanks, that's much less confusing!

So, all of these patches still need a review... The PHP and JS ones should probably be spun off into their own issues, because they will each require some careful review and/or testing, and I'm not sure whether they meet the "beta patch" criteria or not.

Valentine94’s picture

Version: 8.0.x-dev » 7.x-dev
Issue tags: -Needs backport to D7, -Needs reroll
FileSize
240.07 KB

Some back-port for D7.

Status: Needs review » Needs work

The last submitted patch, 69: spellchecking_drupal-2329703-69.patch, failed testing.

Valentine94’s picture

Status: Needs work » Needs review
FileSize
120.49 KB

Try to resolve failing tests.

Valentine94’s picture

Should I divide this patch by some conditions like in #64 #65 #66?

Thanks.

jhodgdon’s picture

Version: 7.x-dev » 8.0.x-dev
Issue tags: +Needs backport to D7

Please read https://www.drupal.org/node/1319154#multiple-versions

We need to resolve Drupal 8 before going to Drupal 7.

mgifford’s picture

Yup! And with the D7 stuff it's going to be a bit of a challenge as we wouldn't want to break any existing sites. Would really limit what we can roll out.

Still, what are the next stages to get this into Core?

Do we need different issues for #64, #65 & #66?

The last submitted patch, 66: drupal8-spellchecked-modules_comments-2329703-66.patch, failed testing.

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

#66 needs a reroll. The other two seem to work just fine still.

rpayanm’s picture

jhodgdon’s picture

As for how to proceed... If someone wants to give the "comments" patch a careful review, that would probably be good, and then we could get that one in if it truly is only affecting comments.

The other two are more problematic. The PHP code one needs to be checked over carefully... but since we at least have a lot of tests for the PHP code, as long as the changes don't change how many test assertions are run we can be fairly certain their probably OK. The JS one is the really dangerous one IMO, because we do not have tests for JS. So someone needs to identify what could be affected and then I guess manually test it.

So yes: let's move at least the JS patch to its own issue, and put it in the JavaScript component, so that the JS maintainers can test/review. I think we can continue to deal with the PHP ones here.

I've also unhidden the "code" patch.

mgifford’s picture

This is so annoying. I almost RTBC'd the comment patch. I'd so like this to just get in quickly..

Sadly, in cases where the php variable is incorrectly spelled, we should probably change the comments and code in the same patch. I looked at every line and grepped for instances where the variable name was spelled wrong (in this patch) and was also wrong in the code. I could only find:

-   * @param \Drupal\Core\Datetime\DateFormatter $date_formater
+   * @param \Drupal\Core\Datetime\DateFormatter $date_formatter

I really think that those should go in the same patch since it makes no sense to change the comments to incorrectly describe the variable.

If that's already been cleared elsewhere, I'm fine with it, but....

rpayanm’s picture

@mgifford wow... nice catch.
Here the patch again...

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

That works for me. #82 is just one part of this issue, but let's see if we can't get it into core.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/modules/comment/src/Tests/CommentNonNodeTest.php
    @@ -194,7 +194,7 @@ function commentContactInfoAvailable() {
    -   * @param boolean $aproval
    +   * @param boolean $approval
    
    +++ b/core/modules/comment/src/Tests/CommentTestBase.php
    @@ -334,7 +334,7 @@ function commentContactInfoAvailable() {
    -   * @param boolean $aproval
    +   * @param boolean $approval
    
    +++ b/core/modules/comment/src/Tests/CommentThreadingTest.php
    @@ -127,7 +127,7 @@ function testCommentThreading() {
    -   * @parm int $cid
    +   * @param int $cid
    
    +++ b/core/modules/system/src/Tests/Entity/EntityAccessControlHandlerTest.php
    @@ -2,7 +2,7 @@
    - * Contains \Drupal\system\Tests\Entity\EntityAccessHControlandlerTest.
    + * Contains \Drupal\system\Tests\Entity\EntityAccessHControlHandlerTest.
    

    These are bugs and should be separated out into a patch that can go in now. The rest of this patch is minor and is subject to https://www.drupal.org/core/beta-changes. The change benefit vs the disruption of this patch just does not play out. Can this be separated out into two issues one to address the PHPDoc bugs and one for the spelling mistakes.

  2. +++ b/core/modules/system/tests/modules/url_alter_test/url_alter_test.install
    @@ -1,7 +1,7 @@
    + * Implement hook_install().
    

    Implements

Status: Needs work » Needs review
jhodgdon’s picture

Please do not requeue these patches. @alexpott asked that this issue be split out into what is actual bugs vs. minor cleanup. We are not going to do the minor cleanup at this time on this scale. It is too disruptive.

So the parts that would actual bugs would be things like:
- @param vs. @parm or other misspellings
- When the documentation does not match the function signature, like @param $fooxyz vs. function($foo). Like here:

-   * @param boolean $aproval
+   * @param boolean $approval
    *   Operation is found on approval page.
    */
   function performCommentOperation($comment, $operation, $approval = FALSE) {

- When a "Contains" in the @file does not match the class name
- Text that would appear in the user interface, with misspelled words in it.

Minor fixes that we need to postpone would include:
- Test method names that have misspelled words in them
- Test assertion message text with misspelled words

jhodgdon’s picture

Actually, documentation fixes are supposed to be prioritized and see #2343099-8: Add @param and @return or fix types in @param and @return in core/lib/Drupal/Core/Database/... apparently "it will conflict with other patches" is not supposed to qualify a patch as 'disruptive'.

So. I think most of the documentation fixes are actually bugs and we should fix them.

mgifford’s picture

I'm for fixing them. They are embarrassing. I can help reroll some of the patches that are affected.

jhodgdon’s picture

Before we reroll any new patches, let's please split this off into one issue per remaining patch.

mgifford’s picture

Issue summary: View changes
mgifford’s picture

I think this is what is needed. Let me know if there is anything missing.

alexpott’s picture

Just filed #2374067: System Module - Corrections as a duplicate.

It also just looks pretty amateur. If this is an enterprise system, we can't be filled with lots of stupid typos in the code.

I take issue with this statement - having seen my fair share of enterprise code I don't think that "stupid typos" in code comments are a high priority for enterprise adoption. User facing text is different and definitely should be a priority.

I can see how we might want to classify a spelling mistake as a bug, fair enough. If it is in a code comment it probably should be a minor bug. Incorrect PHPDoc is more important and should be filed as a normal bug. User facing text is probably a major or critical depending on where the text appears.

I do think that having all the code comments spelt correctly is nice; it shows we care, have a good attention to detail and is way less confusing for people who have a limited proficiency in English.

So in order to make this easier to review and not 100k of change we need to find someway of breaking this up.

mgifford’s picture

A) Short & Sweet Already - #2383865: Spellchecking Drupal - Javascript
B) Minor in consequence but difficult to translate with over 2400 lines in the diff #2383863: Spellchecking Drupal - Comments
C) Not too terribly long, has lots of user facing code, but 45 files & 740 line diff - #2383871: Spellchecking Drupal - PHP

Main concern I think is B. We could separate Views patches. Tests. Would it help if it were just broken down into 5 patches each under 500 lines of code? Just something to make it somewhat less disruptive.

Sorry you didn't like my statement about "enterprise systems". Seems if we are trying to reach out of an English audience, having the comments be at least written in English should help adoption as it's way easier than dealing with the spelling that right now in Core. I can't spell, but think this is something that has largely already been fixed (and folks are asking about back-porting to D7).

Status: Needs review » Needs work

The last submitted patch, 82: drupal8-spellchecked-modules_comments-2329703-82.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
112.89 KB

Here's a re-roll. Nice to see that some spelling errors have been fixed by other patches.

jhodgdon’s picture

I'm confused. We have 3 child issues. What's supposed to be in this patch that's not in the children?

mgifford’s picture

@jhodgdon This has the drupal modules in it. There are only 2 children really. And one duplicate.

I have to look a bit closer at #2383871: Spellchecking Drupal - PHP.

Pere Orga’s picture

Status: Needs review » Needs work
FileSize
8.33 KB

Closing #2488476: Fix 14 typos in code as a duplicate of this one, and adding patch that should be merged with current/new patches.

maximpodorov’s picture

Version: 8.0.x-dev » 7.x-dev

So, Drupal 8 is perfectly spelled now. Let's go back to Drupal 7.

maximpodorov’s picture

Status: Needs work » Needs review
FileSize
218.34 KB

The very first patch is re-rolled, and two typos was already fixed in 7.x.

Pere Orga’s picture

Status: Needs review » Needs work

This should be done in multiple patches too?

Pere Orga’s picture

Status: Needs work » Postponed

Let's postpone this until #2494313: Follow up to Spellchecking Drupal - PHP (affects D7)

Pere Orga’s picture

maximpodorov’s picture

Status: Postponed » Needs review

I don't think this should be postponed now, when the majority of typos in D8 is fixed. New typos can appear in every new commit, and they can't stop fixing D7 which is quite stable (in terms of new large changes).

The last submitted patch, 99: 2488476-fix_14_typos-1.patch, failed testing.

Pere Orga’s picture

Status: Needs review » Needs work

  • jhodgdon committed 2a287f3 on 8.1.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
  • jhodgdon committed 73c66d4 on 8.1.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
  • jhodgdon committed 9cf1459 on 8.1.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
  • jhodgdon committed bedbd0a on 8.1.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
  • jhodgdon committed c1442c5 on 8.1.x
    Issue #2329703 by maximpodorov: Fix spelling in varous files
    
mgifford’s picture

Title: Spellchecking Drupal » [meta] Spellchecking Drupal
Status: Needs work » Fixed

All the child issues are fixed, so this should be too... Although I don't know that it has been backported in all the child issues.

Status: Fixed » Closed (fixed)

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