| Project: | Coder |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | japerry |
| Status: | closed (fixed) |
Issue Summary
Below is the patch against head, that fixes a bunch of issues in the queue, plus many more:
* more drupalapi changes
* one argument drupalapi fixed
* changed drupal_add_css to work properly, along with css
* fix Undefined index in coder_review_page_form() when file doesn't exist (eg: typo)
* fixed parse_ini to drupal_parse_info file in coder_review_7x.inc
* hook drupalapi calls fixed
* hook drupalapi calls fixed
* fix non-existing array in coder_upgrade_conversions_form()
* update phpapi in the theme
* fix Undefined index: dependencies in _coder_review_7x_optional_block_review_callback()
* change theme output for review functions
* updates from d.o #s 565280 565610 569990
* 601372 575738 536194 583136 575734 patches from d.o
* theme changes to d7
* changed the variables[0] to 'form'
* setting default values for hook_theme arguments arrays
* coder_review_theme fix
* theme function changes
This patch should make coder work for drupal 7. We haven't added anything in this patch that changes the rules for conversion found here:
http://drupal.org/node/394070
We still need to tackle the ones that aren't complete yet.
| Attachment | Size |
|---|---|
| coder-head-to-d7.patch | 105.79 KB |
Comments
#1
I stopped reviewing. You can fix below already, but what I really need is diff -up
+++ coder_review/coder_review.css 2009-10-18 02:17:41 +0000@@ -45,3 +45,11 @@
+pre {
+ white-space: pre-wrap; /* css-3 */
+ white-space: -moz-pre-wrap !important; /* Mozilla, since 1999 */
+ white-space: -pre-wrap; /* Opera 4-6 */
+ white-space: -o-pre-wrap; /* Opera 7 */
+ word-wrap: break-word; /* Internet Explorer 5.5+ */
+}
What's that? Please remove.
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -539,10 +541,12 @@
- $rows[$row][$col] = drupal_render($element);
+ $rows[$row][$col] = drupal_render($element);
Please revert.
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -539,10 +541,12 @@
- return theme('table', array(), $rows);
+
+ $output = theme('table', array('rows' => $rows, 'attributes' => array('id' => 'filter-order')));
+ return $output;
We can return directly here.
I'm on crack. Are you, too?
#2
updated per requests
#3
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -730,8 +734,7 @@ function coder_review_page_form($form, &
+ drupal_add_css(drupal_get_path('module', 'coder_review') . '/coder_review.css');
We can use #attached here.
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -819,7 +822,7 @@ function coder_review_page_form($form, &
- '#markup' => theme('coder_review', $name, $filename, $results),
+ '#markup' => theme('coder_review', array('name' => $name, 'filename' => $filename, 'results' => $results)),
@@ -952,7 +955,7 @@ function _coder_review_page_form_include
- '#markup' => theme('coder_review', $name, $filename, $results),
+ '#markup' => theme('coder_review', array('name' => $name, 'filename' => $filename, 'results' => $results)),
Can't we use #theme here now?
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -1771,6 +1778,27 @@ function do_coder_review_callback(&$code
/**
+ * Check for the presence of a hook.
+ *
+ * @note
+ * See do_coder_review_regex() for arguments.
+ */
+function do_coder_review_hook(&$coder_args, $review, $rule, $lines, &$results) {
Could use some more docs/description about the purpose of this function.
Use @see to refer to other functions.
@note is a unsupported directive.
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -1880,11 +1908,42 @@ function coder_review_simpletest() {
+ 'name' => NULL,
+ 'filename' => NULL,
+ 'results' => NULL,
...
+ 'warning' => NULL,
+ 'severity_name' => NULL,
+ 'lineno' => NULL,
+ 'line' => NULL,
(and elsewhere) Please don't align these in a column.
+++ coder_review/coder_review.module 2009-10-18 03:20:51 +0000@@ -1920,10 +1983,17 @@ function theme_coder_review($name, $file
+ return theme('coder_review_warning', array(
...
+ 'line' => $error['line']
+ ));
@@ -1959,10 +2029,20 @@ function _coder_review_warning($rule) {
+ return theme_drush_coder_review_warning(array(
...
+ 'line' => $line
+ ));
@@ -1986,9 +2066,14 @@ function theme_coder_review_warning($war
+ $img = theme('image', array(
...
+ 'getsize' => FALSE
+ ));
Missing trailing comma.
+++ coder_review/includes/coder_review_7x.inc 2009-10-18 03:20:51 +0000@@ -190,8 +191,8 @@ function coder_review_7x_reviews() {
array(
- '#type' => 'regex',
- '#value' => 'function\s+[a-z0-9_]+_(access)\s*\(',
+ '#type' => 'hook',
+ '#value' => 'access',
'#function-not' => '^\w+_node_access$',
'#warning_callback' => '_coder_review_7x_hook_access_warning',
),
@@ -938,14 +960,45 @@ function _coder_review_7x_drupal_behavio
+function _coder_review_7x_optional_block_review_callback(&$coder_args, $review, $rule, $lines, &$results) {
(and elsewhere) New review rules do not belong into this patch.
+++ coder_review/includes/coder_review_style.inc 2009-10-17 21:09:06 +0000@@ -26,13 +26,13 @@ function coder_review_style_reviews() {
- '#value' => '\s(if|elseif|while|foreach|switch|return|for|catch)\(',
+ '#value' => '\s(if|elseif|while|foreach|switch|case|return|for|catch)\(',
...
- '#not' => '^(if|elseif|while|foreach|switch|return|for|list|catch)$',
+ '#not' => '^(if|elseif|while|foreach|switch|case|return|for|list|catch)$',
Ditto.
+++ coder_review/tests/coder_review_format.test 2009-10-17 21:09:06 +0000@@ -7,8 +7,58 @@
+class CoderReviewFormatTest extends CoderReviewTestCase {
Hmmmm.... I thought I committed these test updates for coder_format a long time ago?
I'm on crack. Are you, too?
#4
new update of the diff....
addressed the issues above, except for the following
+++ coder_review/includes/coder_review_7x.inc 2009-10-18 03:20:51 +0000@@ -190,8 +191,8 @@ function coder_review_7x_reviews() {
array(
- '#type' => 'regex',
- '#value' => 'function\s+[a-z0-9_]+_(access)\s*\(',
+ '#type' => 'hook',
+ '#value' => 'access',
'#function-not' => '^\w+_node_access$',
'#warning_callback' => '_coder_review_7x_hook_access_warning',
),
That change is not adding a rule but porting a change from the existing rule.
#5
current working bzr of the d7 port:
https://code.launchpad.net/~d7cx/d7cx-coder/DRUPAL-7--1
Direct tar.gz download: http://launchpad.net/d7cx-coder/trunk/coder-d7-beta1/+download/coder-7.x...
#6
That was, somehow, the same patch again? (same size, etc?)
#7
First of all, thanks! However rather than one big patch merging in a whole bunch of things from different issues, I would have much rather seen many smaller patches, each dealing with one issue or one type of issue. One large patch covering so many different items makes it much harder to review.
I'm reviewing the patch now, but I already see items that will be dropped and handled separately in other patches / issues. For example, there's a fair number of new reviews in there which shouldn't really be in a patch to make coder work with latest D7 HEAD. It also includes the patch from #575738: false positives for hook sniffing which isn't the right solution as it won't work in every scenario (or even most of them), so that's going to have to be dealt with separately and in the original issue opened for it.
#8
Okay this is what I committed:
- changed menu paths so it is more in line with new D7 menu structure
- updated arguments to theme() function calls and definitions.
- changed drupal_add_css/js calls to use #attached instead.
- changed #markup theme() calls to use #theme.
- coding style fixes.
- added CHANGELOG.txt files
I did not commit:
- the new reviews
- the patch for #575738
- the css change : can you explain why this is needed?
- the change to coder_upgrade
- changes to coder_format - sun you may want to review those changes first
These can be dealt with in other issues.
I've attached the patch in case you want to see what was added.
Thanks for all your hard work! I appreciate it.
Cheers,
Stella
#9
Automatically closed -- issue fixed for 2 weeks with no activity.
#10
Noticed hook_theme() was a bit wonky, causing unknown index errors all over, so I fiddled around with it until I had a result I could use to help me start porting themes to D7
still getting a ton of errors like
Warning: parse_ini_file(sites/all/themes/fusion/fusion_core/template.info): failed to open stream: No such file or directory in _coder_review_7x_optional_block_review_callback() (line 999 of C:\xampp\htdocs\fusion\sites\all\modules\coder\coder_review\includes\coder_review_7x.inc).which I don't rly understand as this is suppose to be template.php, not template.info
but at least now I get somewhat of a review :D
I hope these changes make sense and I'm affraid the rest is out of my skill level
#11
#seutje - What you're noticing seems to be the same as #615786: Update according to hook_theme api changes