Problem/Motivation
./rules_i18n/rules_i18n.i18n.inc:14: Overriden ==> Overridden
./rules_i18n/rules_i18n.i18n.inc:25: Overriden ==> Overridden
./modules/rules_core.eval.inc:234: Overriden ==> Overridden
./ui/rules.autocomplete.js:84: overriden ==> overridden
./ui/ui.core.inc:460: Additionaly ==> Additionally
./includes/rules.core.inc:1741: overriden ==> overridden
./includes/rules.plugins.inc:327: Overriden ==> Overridden
./includes/rules.processor.inc:269: Overriden ==> Overridden
./includes/rules.processor.inc:287: Overriden ==> Overridden
./includes/faces.inc:238: overriden ==> overridden
./rules_scheduler/rules_scheduler.admin.inc:119: paramter ==> parameter
./tests/rules.test:171: overriden ==> overridden
./tests/rules.test:1224: infinte ==> infinite
./tests/rules.test:1399: comparision ==> comparison
| Comment | File | Size | Author |
|---|---|---|---|
| #32 | 2859698-32.patch | 33.37 KB | tr |
| #29 | 2859698-29.patch | 33.36 KB | tr |
| #21 | interdiff_20-21.txt | 716 bytes | mcdruid |
| #21 | clean_rules_module_by-2859698-21.patch | 33.82 KB | mcdruid |
| #20 | interdiff_18-20.txt | 1.18 KB | dhruveshdtripathi |
Comments
Comment #2
dbjpanda commentedCorrected all typos. Please review.
Comment #3
dbjpanda commentedComment #4
krina.addweb commentedhi @dbjpanda I add some issues of typo below, I not being able to complete all files but there is the need for it.
& thanks for the patch it works well as described.
Comment #5
dbjpanda commented@krina.addweb plz check the latest patch. Updated as per your concern
Comment #6
dbjpanda commentedComment #7
dbjpanda commentedThis one is the final with one more. Fugly => ugly. Kindly review. Uploaded the #7 patch with inter-diff between #2 and #7
Comment #8
dbjpanda commentedComment #9
dbjpanda commentedComment #10
rodrigoac commentedComment #11
rodrigoac commentedHi people!
I gave a double check in all files, and find some things that we also need to fix.
Thanks a lot !
Comment #12
smaaz commentedHello,
Fixed all the typos as per suggestion in #11.
Attached is the patch file.
Comment #13
dbjpanda commentedComment #14
rodrigoac commentedHi Guys,
I change the status to needs review, because the patch #12 was generated without the patch #7, so I merge both, and correct some typos that I find when I was doing a review.
Thanks for the effort.
Comment #15
dbjpanda commented@rodrigoac Thanks for pointing out. RTBC
Comment #16
mcdruid commentedConfirmed that the patch in #14 applies cleanly to 7.x-2.x
Noting that there is one change included which is not in text intended for human consumption:
So that'll affect functionality. However, it looks like a legitimate bug fix AFAICS.
Comment #17
fagoPatch looks really great, however this is a small issue:
This exceeds on 80chars now.
Comment #18
rpayanmComment #19
mcdruid commentedCouple more nitpicks I'm afraid:
...that also pushes the line over 80 chars.
...we may as well fix the sniff violations here that there should be spaces before the comments i.e.
You could argue that last one is scope creep, but as we're changing one of the lines anyway...
Comment #20
dhruveshdtripathi commentedMade changes suggested in comment #19. Patch and interdiff attached.
Comment #21
mcdruid commentedThanks! There was actually one more change which pushed a comment over 80 chars (which I missed in #19), so here's another patch which fixes that one.
If the tests pass for these (as we'd expect them to), I'd say we're hopefully RTBC.
Comment #23
mcdruid commentedLooks like we're hitting d.o CI errors. Will try to get a re-test later.
Comment #24
mcdruid commentedBack to NR for a retest.
Comment #25
mcdruid commentedOk, tests passed this time. Back to RBTC.
Comment #26
tr commentedI have reviewed this patch and I think all the changes are good and appropriate.
The patch still applies cleanly and the tests run green.
There are a few places where translated strings have been modified. In core, this would not be allowed in a minor point release because it breaks the existing translations of this module. Some contrib uses that same standard, some doesn't. It would be reasonable to ask that those modifications are removed from the patch, or not removed, depending on how the maintainer feels about this.
A lot of people have worked on this.
Maintainer: Please either commit the patch, provide feedback and guidance for further changes to the patch, or mark it as "Won't fix".
Comment #27
tr commentedBump.
Patch still applies and passes the tests.
Comment #28
fagoI'm sry for letting this nice cleanup lie so long, however it does not apply any more now. Could some reroll it?
Comment #29
tr commentedHere's a re-roll. The patch failed to apply because one of the fixes was already made in the just-committed #2952654: Support PHP 7.2.
Comment #30
tr commentedMoving back to RTBC because this was just a simple re-roll.
Comment #32
tr commentedNeeds re-roll because #2297087: Typo in RulesPluginFeaturesIntegrationInterace has been committed. Here is the new patch:
Comment #33
tr commentedBack to RTBC after simple re-roll.
Comment #35
tr commentedCommitted. Thanks to all who helped!
If there are any further problems that this issue missed, please open a new issue.
Comment #36
tr commentedNote: I checked 8.x-3.x, and none of these corrections need to be ported to D8.