Posted by sun on May 14, 2009 at 7:44pm
Jump to:
| Project: | Pathologic |
| Version: | 6.x-1.0-beta18 |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Same module, just clean, so I can review.
| Attachment | Size |
|---|---|
| pathologic-DRUPAL-6--1.cleanup.patch | 13.17 KB |
Comments
#1
bump
#2
Playing Patch Bingo I found this.
I let coder run on the module and looked at the code. There are plenty of warnings like
Line 17: string concatenation should be formatted without a space separating the operators (dot .) and a quote
if (variable_get('filter_pathologic_href_' . $format, TRUE)) {
But I think this is not much of a problem. The Issues of the patch seems to be solved.
So I am marking this as fixed so its out of patch Bingo.
#3
Thanks - however, I'm pretty sure that Coder does not cover the clean-ups of this patch.
#4
I applied this patch a long time ago, and have built on it since then. So closing it again.
With regards to the placement of the dot, it was part of Drupal's coding standards to put the dot up against string literals when concatenating; one I freely ignored because it was stupid. (Why is $foo + 5 correct, but $foo . 'bar' wrong?) Spaces on both sides of the dot is now correct in D7, so I won't bother "fixing" it with the D6 version of my module.
#5
That was quick! :D
Okay my fault. There was a lot more in your patch than just eof and tabs. But I get a grip to all of this and hope this is a better review.
All tabs and eof seems to be fixed. The comments seem to be in correct format now.
But the ; $Id$ is still missing in some files.
The beta22 did change a lot as far as I can see. So please reroll your patch agains the latest version.
+++ README.txt 14 May 2009 19:33:41 -0000@@ -15,4 +17,4 @@ Installation & Configuration
-Distributed under the GPL v2 license, under protest.
\ No newline at end of file
+Distributed under the GPL v2 license, under protest.
Line ends seem fine to me
+++ pathologic.info 14 May 2009 19:33:26 -0000@@ -1,6 +1,7 @@
-package = "Input filters"
+package = Input filters
missing
+++ pathologic.info 14 May 2009 19:33:26 -0000@@ -1,6 +1,7 @@
-php = 4.4
\ No newline at end of file
+php = 4.4
Line end seems fine to me
+++ pathologic.module 14 May 2009 19:40:01 -0000@@ -2,7 +2,8 @@
- * @defgroup pathologic Pathologic text filter for Drupal
+ * @file
+ * Pathologic text filter for Drupal.
*
* This input filter attempts to make sure that link and image paths will
* always be correct, even when domain names change, content is moved from one
fixed
+++ pathologic.module 14 May 2009 19:40:01 -0000@@ -10,127 +11,121 @@
- if ($op === 'process' && $text !== '') {
- if (variable_get("filter_pathologic_href_{$format}", TRUE)) {
- // Do transformation on HREF paths
- // Make relative
All tabs are removed
+++ pathologic.module 14 May 2009 19:40:01 -0000@@ -10,127 +11,121 @@
- * Return the hard part of the regular expression to be used when making paths
- * relative. $attr will probably be either 'href' or 'src'.
+ * Return the hard part of the regular expression to be used when making paths relative.
+ *
+ * $attr will probably be either 'href' or 'src'.
All functions descriptions were updated
+++ README.txt 14 May 2009 19:33:41 -0000@@ -1,3 +1,5 @@
+# $Id$
+
missing
I'm on crack. Are you, too?
#6
Stop reviewing months old code, please. If you want to review my module, review the actual module as available today.
#7
Automatically closed -- issue fixed for 2 weeks with no activity.