Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ashwinsh’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.25 KB

Created a path for the same.

Status: Needs review » Needs work

The last submitted patch, 1: 1844266-Spelling-mistake.patch, failed testing. View results

ashwinsh’s picture

Status: Needs work » Needs review
ashwinsh’s picture

jweowu’s picture

Status: Needs review » Reviewed & tested by the community
TR’s picture

Version: 7.x-1.0-rc3 » 7.x-1.x-dev
FileSize
7.88 KB

Well if it's going to take 9 years just to correct spelling, we might as well do it all at once. Here's a fix for all the spelling errors reported by codespell, which includes the error from above.

jweowu’s picture

Title: Spelling mistake » Fix multiple spelling mistakes

Cheers TR. By way of explanation, this was one of multiple patches I'd created after I'd spotted a spelling mistake in one module and then searched for and found the same mistake in several other modules I was using. I hadn't gone looking for additional errors in any of them, but you've certainly turned up a bunch!

Looks good to me. Confirming as still RTBC.

Liam Morland’s picture

It would be helpful to put this patch into an issue fork.

jweowu’s picture

Feel free to do that. The traditional approach to issue queue patches is not in any way deprecated (to my knowledge), so I'm not sure how it would help; but if you believe it would make it more likely for this to be fixed then go for it.

jweowu’s picture

Component: Views integration » Code - misc
Issue summary: View changes
Liam Morland’s picture

Using an issue fork means that merging is one click and re-rolling after other commits is often one click as well. Patches that do not apply are a thing of the past.

In the DrupalCon North America 2021 Driesnote, he proposed more fully adopting GitLab and moving away from current issue queues, so the patch workflow may be deprecated shortly.

Though the most important thing that we need is an active maintainer for D7 in this module.

TR’s picture

Unfortunately, the spelling mistakes in the original post are in translatable strings. Fixing these mistakes will break translations. That's not something we can do in a minor point release of Rules. So we're stuck with those errors until D7 EOL.

The other mistakes are in comments, so they can be fixed now. Here's a new patch with only the "fixable" mistakes.

  • TR committed 068a010 on 7.x-1.x
    Issue #1844266 by TR, jweowu: Fix multiple spelling mistakes
    
TR’s picture

Status: Reviewed & tested by the community » Fixed

Committed #12.

Status: Fixed » Closed (fixed)

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

jweowu’s picture

Unfortunately, the spelling mistakes in the original post are in translatable strings. Fixing these mistakes will break translations. That's not something we can do in a minor point release of Rules. So we're stuck with those errors until D7 EOL.

The notion of being "stuck with spelling mistakes" seems much too silly to be true. Surely English translations can be added for the misspelled text (translating to the correct spelling) ?

That is definitely possible via $conf['locale_custom_strings_en'] so I'd be surprised if it wasn't possible via other translation systems as well.

TR’s picture

Yes, it's been very frustrating dealing with this in Drupal over the years. The rules are that breaking changes can't be made between minor point releases. That has meant that changes to translatable strings can't be made until a new major release. That has been true of D5, D6, and D7. The situation has improved in D8, but there are still considerations on when changes like this may be introduced.

There are workarounds. You can patch your own site, or you can use the string_translations module to essentially translate English-to-English, or you can use variable overrides like you suggest, or you can hook_form_alter() things, but what you can't do is change a translatable string directly in the code because then all the sites using languages other than English will suddenly see that string as untranslated.

If we change translatable strings now, that creates an admittedly minor problem for hundreds of thousands of sites that either use translations or have taken steps to correct this problem on their own. But it does create a problem, and that's not something I want to do for a supposedly stable and near-EOL D7 release. I would prefer that this module just continue to work as it always has, rather than surprise people by introducing an error on their sites.

I personally would have committed this change 9 years ago when the problem was first raised, and made an effort at that time to check on ALL translatable strings. That would have broken some sites, but back then D7 was cutting edge, this module was being actively developed and wasn't on as many sites, and people expected changes between versions. Now, with the module stagnant for many years, people expect stability and aren't likely to either update or to notice the breakage when they do update.

jweowu’s picture

Fair enough then. Thank you for taking the time to elaborate.