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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dbjpanda created an issue. See original summary.

dbjpanda’s picture

Corrected all typos. Please review.

dbjpanda’s picture

Status: Active » Needs review
krina.addweb’s picture

Status: Needs review » Needs work

hi @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.

rules.autocomplete.js
* Toogle the autcomplete window.
Change it to "Toggle" & "autocomplete"

rules.ui.css
line 89
/* The span element with the icon which opens the log, has a whitepsace.
Change "whitepsace" to "whitespace"

ui.controller.inc
line 38
// for base paths like admin/config/workflow/rules. Therefor we have to
change "Therefor" to "Therefore"

line 160
* Generates the render array for a overview configuration table for arbitrary
change "a" to "an"

ui.core.inc
line 260
if (!empty($form['provides'])) {
      $help = '<div class="description">' . t('Adjust the names and labels of provided variables, but note that renaming of already utilizied variables 
Change the spelling of "utilizied"

line 460
else {
        $description = t('Variables are normally input <em>parameters</em> for the component – data that should be available for the component to act on. Additionaly, action components may <em>provide</em> variables back to the caller. Each variable must have a specified data type, a label and a unique machine readable name containing only lowercase alphanumeric characters and underscores. See <a href="@url">the online documentation</a> for more information about variables.',
Change "Additionaly" to "Additionally"

line 672
*   The name of the data typ
change "typ" to "type"

line 1316
// Verfiy the current user has access to use it.
Change "Verfiy" to "Verify"

ui.data.inc
line 373
//the timpeicker is available.	
change "timpeicker" to "timepicker"

ui.forms.inc
line 294
* given implemenation name and rebuild the form.
Change "implemenation" to "implementation"

line 490
* Form to remove a event from a rule.
Change "a" to "an"

line 699
* correct and converts date values specifiy a fixed (= non relative) date to
change "specifiy" to "Specify"

rules.test
line 391
// Reset global user to anonyous.
change "anonyous" to "anonymous"

line 1224
* rule itself - thus we won't run in an infinte loop.
change "infinte" to "infinite"
							
line 1399
$this->assertEqual($node->title, 'bar', "Data comparison using IN operation evaluates to TRUE.");
"comparision" to "comparison"
 							
line 1471
$this->assertEqual($result, 9, 'Converted decimal to integer using roundin behavio down.');
change "roundin" to "rounding" & "behavio" to "behaviour"

line 1608
$this->assertEqual(array_values($node->field_tags[LANGUAGE_NONE]), array(0 => array('tid' => $tid)), 'Entity has field conditions evaluted.');
"evaluted"	to "evaluated"

line 1986
// Remove the authenticate role so we only use the new role created by
change "authenticate" to "authenticated"
						
dbjpanda’s picture

@krina.addweb plz check the latest patch. Updated as per your concern

dbjpanda’s picture

Status: Needs work » Needs review
dbjpanda’s picture

This one is the final with one more. Fugly => ugly. Kindly review. Uploaded the #7 patch with inter-diff between #2 and #7

dbjpanda’s picture

dbjpanda’s picture

Issue tags: +Novice
rodrigoac’s picture

Assigned: Unassigned » rodrigoac
rodrigoac’s picture

Assigned: rodrigoac » Unassigned
Status: Needs review » Needs work

Hi people!

I gave a double check in all files, and find some things that we also need to fix.

Thanks a lot !

include/faces.inc
line 150
* Returns whether the object can face as the given interface, thus it
* returns TRUE if this oject has been extended by an appropriate
* implementation.
change "oject" to "object"

line 154
Optional. A interface to test for. If it's omitted, all interfaces that
*   the object can be faced as are returned.
Change "A interface" to "An interface"

includes/rules.core.info
line 116
    // The key ist the configuration name and the value the actual export.
Change "ist" to "is"

line 535
   * configuration. To iterate over the children only, just regulary iterate
Change "regulary" to "regularly"

line 848
   *   In case of a failed integraty check, a RulesIntegrityException exception
Change "integraty" to "integrity"

line 1256
   * Seamlessy invokes the method implemented via faces without having to think
Change "Seamlessy" to "Seamlessly" 

line 1347
   *   The exported confiugration.
Change "confiugration" to "configuration" 

line 1397
   * Finalizies the configuration export by adding general attributes regarding
Change "Finalizies" to "Finalizes"

line 2315
    // weight by ensuring later childrens would have higher weights.
Change "childrens" to "children" 

includes/rule.plugins.inc
line 13
   * implements the ArrayAcces interface.
Change "ArrayAcces" to "ArrayAccess"

includes/rules.processor.inc
   * Prepares the evalution, e.g. to determine whether the input evaluator has
Change "evalution" to "evaluation" 

includes/rules.state.inc
line 239
   * Merged in saves are removed from the given state, but not mergable saves
Change "mergable" to "mergeable"

line 294
          // Make sure we are usign the right language. Wrappers might be cached
Change "usign" to "using" 

line 588
        // Apply any 'type' and 'bundle' assertion directly to the propertyinfo.
Change "propertyinfo" to "property info"

line 724 
   * Prepare for serializiation.
Change "serializiation" to "serialization" 

includes/rules.upgrade.inc
line 31
      t('In order to convert a rule or rule set make sure you have all dependend modules installed and upgraded, i.e. modules which provide Rules integration that has been used in your rules or rule sets. In addition those modules may need to implement some Rules specific update hooks for the conversion to properly work.') . ' ' .
Change "dependend" to "dependent" 

line 560
  return 'breadcumb_set';
Change "breadcumb_set" to "breadcrumb_set"

modules/php.eval.inc
line 133
 * Evalutes the given PHP code, with the given variables defined.
Change "Evalutes" to "Evaluates"

modules/rules_core.eval.inc
line 49
    // Cleanup the state, what saves not mergable variables now.
Change "mergable" to "mergeable" 

tests/rules.test
line 1513
    // Test entiy_is_of_bundle condition.
Change "entiy_is_of_bundle" to "entity_is_of_bundle"

ui/ui.core.inc
line 948
    // Recurse over all element childrens or use the provided iterator.
Change "childrens" to "children"

line 968
        // If another iterator is passed in, the childs parent may not equal
Change "childs" to "child" 

README.TXT
line 88
    plans to do so, as unfortuntely the module's design does not allow for
Change "unforuntly" to "unforunately" 

rules.api.php
line 179
 *     a previsouly used 'group'.
Change "previsouly" to "previously" 

line 263
 *   The action may return an array containg parameter or provided variables
Change "containg" to "containing"

line 265
 *   provdide the value for a provided variable.
Change "provdide" to "provide" 

line 449
 *     For modules implementing identifiable data types being non-entites the
Change "non-entites" to "non-entities" 

line 999 
 * configruation of the Drupal 6 action ($element) to the Drupal 7 version
Change "configruation" to "configuration"

rules.features.inc
line 38
          // Directly use __call() so we cann pass $export by reference.
Change "cann" to "can"

rules.modules
line 100
 *   available for the configuraion elements. Values for the variables need to
Change "configurarion" to "configuration"

line 160
 *     'item:label': Optionally a lebel for the list item variable.
Change "lebel" to "label"

line 245
 * Used for collecting events, rules, actions and condtions from other modules.
Change "condtion" to "condition"

line 929
  // oftent used components.
Change "oftent" to "often" 
smaaz’s picture

Status: Needs work » Needs review
FileSize
16.31 KB

Hello,

Fixed all the typos as per suggestion in #11.

Attached is the patch file.

dbjpanda’s picture

Title: Correct some typos » Clean "Rules" module by correcting all typos and grammars
Status: Needs review » Reviewed & tested by the community
rodrigoac’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
33.49 KB

Hi 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.

dbjpanda’s picture

Status: Needs review » Reviewed & tested by the community

@rodrigoac Thanks for pointing out. RTBC

mcdruid’s picture

Confirmed that the patch in #14 applies cleanly to 7.x-2.x

rules$ git status
On branch 7.x-2.x
Your branch is up-to-date with 'origin/7.x-2.x'.

rules$ git apply clean_rules_module_by-2859698-14.patch

rules$ git diff --stat
 README.txt                                |  2 +-
 includes/faces.inc                        |  6 +++---
 includes/rules.core.inc                   | 16 ++++++++--------
 includes/rules.plugins.inc                |  4 ++--
 includes/rules.processor.inc              |  6 +++---
 includes/rules.state.inc                  |  8 ++++----
 includes/rules.upgrade.inc                |  4 ++--
 modules/php.eval.inc                      |  4 ++--
 modules/rules_core.eval.inc               |  4 ++--
 modules/system.eval.inc                   |  4 ++--
 rules.api.php                             | 10 +++++-----
 rules.features.inc                        |  2 +-
 rules.module                              |  8 ++++----
 rules_i18n/rules_i18n.i18n.inc            |  4 ++--
 rules_i18n/rules_i18n.test                |  2 +-
 rules_scheduler/rules_scheduler.admin.inc |  2 +-
 rules_scheduler/rules_scheduler.test      |  2 +-
 tests/rules.test                          | 14 +++++++-------
 ui/rules.autocomplete.js                  |  4 ++--
 ui/rules.ui.css                           |  2 +-
 ui/ui.controller.inc                      |  6 +++---
 ui/ui.core.inc                            | 12 ++++++------
 ui/ui.data.inc                            |  2 +-
 ui/ui.forms.inc                           |  6 +++---
 24 files changed, 67 insertions(+), 67 deletions(-)

Noting that there is one change included which is not in text intended for human consumption:

--- a/includes/rules.upgrade.inc
+++ b/includes/rules.upgrade.inc

...snip...

@@ -557,7 +557,7 @@ function rules_action_save_variable_upgrade($element, $target) {
 
 // System.module integration.
 function rules_action_set_breadcrumb_upgrade_map_name($element) {
-  return 'breadcumb_set';
+  return 'breadcrumb_set';
 }

So that'll affect functionality. However, it looks like a legitimate bug fix AFAICS.

fago’s picture

Status: Reviewed & tested by the community » Needs work

Patch looks really great, however this is a small issue:

+++ b/includes/faces.inc
@@ -147,11 +147,11 @@ if (!class_exists('FacesExtendable', FALSE)) {
+     *   Optional. An interface to test for. If it's omitted, all interfaces that

This exceeds on 80chars now.

rpayanm’s picture

Status: Needs work » Needs review
FileSize
33.56 KB
mcdruid’s picture

Status: Needs review » Needs work

Couple more nitpicks I'm afraid:

+++ b/ui/ui.controller.inc                                                         
                     
@@ -157,7 +157,7 @@ class RulesUIController {                                      
   }                                                                               
                                                                                   
   /**                                                                             
-   * Generates the render array for a overview configuration table for arbitrary
+   * Generates the render array for an overview configuration table for arbitrary
    * rule configs that match the given conditions.                             

...that also pushes the line over 80 chars.

+++ b/ui/ui.data.inc                                                               
@@ -370,7 +370,7 @@ class RulesDataUIDate extends RulesDataUIText {                
             '!strtotime' => l('strtotime()', 'http://php.net/strtotime')));       
                                                                                   
     //TODO: Leverage the jquery datepicker+timepicker once a module providing  
-    //the timpeicker is available.                                                
+    //the timepicker is available.                                                
     return $form;

...we may as well fix the sniff violations here that there should be spaces before the comments i.e.

   372 | ERROR   | [x] No space found before comment text; expected "//           
       |         |     TODO: Leverage the jquery datepicker+timepicker            
       |         |     once a module providing" but found "//TODO:                
       |         |     Leverage the jquery datepicker+timepicker once a           
       |         |     module providing"                                          
   373 | ERROR   | [x] No space found before comment text; expected "//           
       |         |     the timepicker is available." but found "//the             
       |         |     timepicker is available."

You could argue that last one is scope creep, but as we're changing one of the lines anyway...

dhruveshdtripathi’s picture

Status: Needs work » Needs review
FileSize
33.81 KB
1.18 KB

Made changes suggested in comment #19. Patch and interdiff attached.

mcdruid’s picture

Thanks! 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.

Status: Needs review » Needs work

The last submitted patch, 21: clean_rules_module_by-2859698-21.patch, failed testing. View results

mcdruid’s picture

Looks like we're hitting d.o CI errors. Will try to get a re-test later.

mcdruid’s picture

Status: Needs work » Needs review

Back to NR for a retest.

mcdruid’s picture

Status: Needs review » Reviewed & tested by the community

Ok, tests passed this time. Back to RBTC.

TR’s picture

I 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".

TR’s picture

Bump.

Patch still applies and passes the tests.

fago’s picture

Status: Reviewed & tested by the community » Needs work

I'm sry for letting this nice cleanup lie so long, however it does not apply any more now. Could some reroll it?

TR’s picture

Here'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.

TR’s picture

Status: Needs review » Reviewed & tested by the community

Moving back to RTBC because this was just a simple re-roll.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 29: 2859698-29.patch, failed testing. View results

TR’s picture

Status: Needs work » Needs review
FileSize
33.37 KB

Needs re-roll because #2297087: Typo in RulesPluginFeaturesIntegrationInterace has been committed. Here is the new patch:

TR’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC after simple re-roll.

  • TR committed c376fdc on 7.x-2.x
    Issue #2859698 by dbjpanda, TR, mcdruid, dhruveshdtripathi, rodrigoac,...
TR’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks to all who helped!

If there are any further problems that this issue missed, please open a new issue.

TR’s picture

Note: I checked 8.x-3.x, and none of these corrections need to be ported to D8.

Status: Fixed » Closed (fixed)

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