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 CreditAttribution: dbjpanda commentedCorrected all typos. Please review.
Comment #3
dbjpanda CreditAttribution: dbjpanda commentedComment #4
krina.addweb CreditAttribution: krina.addweb at AddWeb Solution Pvt. Ltd. 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 CreditAttribution: dbjpanda commented@krina.addweb plz check the latest patch. Updated as per your concern
Comment #6
dbjpanda CreditAttribution: dbjpanda commentedComment #7
dbjpanda CreditAttribution: dbjpanda at Google Summer of Code 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 CreditAttribution: dbjpanda commentedComment #9
dbjpanda CreditAttribution: dbjpanda commentedComment #10
rodrigoac CreditAttribution: rodrigoac at CI&T commentedComment #11
rodrigoac CreditAttribution: rodrigoac at CI&T 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 CreditAttribution: smaaz as a volunteer and at InfoBeans Technologies Limited commentedHello,
Fixed all the typos as per suggestion in #11.
Attached is the patch file.
Comment #13
dbjpanda CreditAttribution: dbjpanda commentedComment #14
rodrigoac CreditAttribution: rodrigoac at CI&T 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 CreditAttribution: dbjpanda commented@rodrigoac Thanks for pointing out. RTBC
Comment #16
mcdruidConfirmed 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
mcdruidCouple 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 CreditAttribution: dhruveshdtripathi at DevsAdda commentedMade changes suggested in comment #19. Patch and interdiff attached.
Comment #21
mcdruidThanks! 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
mcdruidLooks like we're hitting d.o CI errors. Will try to get a re-test later.
Comment #24
mcdruidBack to NR for a retest.
Comment #25
mcdruidOk, tests passed this time. Back to RBTC.
Comment #26
TR CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: TR commentedMoving back to RTBC because this was just a simple re-roll.
Comment #32
TR CreditAttribution: TR commentedNeeds re-roll because #2297087: Typo in RulesPluginFeaturesIntegrationInterace has been committed. Here is the new patch:
Comment #33
TR CreditAttribution: TR commentedBack to RTBC after simple re-roll.
Comment #35
TR CreditAttribution: 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 CreditAttribution: TR commentedNote: I checked 8.x-3.x, and none of these corrections need to be ported to D8.