Coding standards

What's to anwser? You reported an issue. Noone has worked on it.

It's still an issue in the latest code too. This should be fixed at least when the code is being reviewed for D7 migration, then backported to D6.

drush --contrib --no-empty --upgrade7x coder-review

sites/all/modules/contrib/devel/devel_generate/devel_generate.module:
+13: [critical] Module .info files must now specify all loadable code files explicitly.

sites/all/modules/contrib/devel/devel_generate/devel_generate.inc:
+131: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME
+135: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME
+213: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME
+265: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME

sites/all/modules/contrib/devel/devel_node_access.module:
+13: [critical] Module .info files must now specify all loadable code files explicitly.
+100: [critical] Parameters to hook_form_alter http://api.drupal.org/api/function/hook_form_alter/7() have changed.
+516: [critical] Context has been added to t http://api.drupal.org/api/function/t/7() as the third parameter, locale has to be an element in the array. Example: array("context" => "frontpage", "locale" => "de").
+1088: [critical] hook_access http://api.drupal.org/api/function/hook_access/6() removed in favor of hook_node_access http://api.drupal.org/api/function/hook_node_access/7().

sites/all/modules/contrib/devel/devel.admin.inc:
+111: [critical] Context has been added to t http://api.drupal.org/api/function/t/7() as the third parameter, locale has to be an element in the array. Example: array("context" => "frontpage", "locale" => "de").

sites/all/modules/contrib/devel/devel.drush.inc:
+121: [critical] Use absolute path, constructed from DRUPAL_ROOT, when including a file.
+139: [critical] Use absolute path, constructed from DRUPAL_ROOT, when including a file.

sites/all/modules/contrib/devel/devel.js:
File: [critical] JavaScript should be compatible with other libraries than jQuery.

sites/all/modules/contrib/devel/devel_generate/devel_generate.inc:
+131: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME
+135: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME
+213: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME
+265: [critical] For improved performance, it is highly recommended that time http://php.net/time() is replaced with REQUEST_TIME

sites/all/modules/contrib/devel/devel_krumo_path.js:
File: [critical] JavaScript should be compatible with other libraries than jQuery.

sites/all/modules/contrib/devel/devel_node_access.js:
File: [critical] JavaScript should be compatible with other libraries than jQuery.
+24: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".

sites/all/modules/contrib/devel/jquery-1.4.4-uncompressed.js:
File: [critical] JavaScript should be compatible with other libraries than jQuery.
+5217: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5612: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5620: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5695: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5705: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5709: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5720: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5730: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5747: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+5762: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+6083: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+6144: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+6267: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".
+6281: [critical] Changed Drupal.behaviors to objects having the methods "attach" and "detach".

sites/all/modules/contrib/devel/krumo/docs/media/lib/classTree.js:
File: [critical] JavaScript should be compatible with other libraries than jQuery.

sites/all/modules/contrib/devel/krumo/krumo.js:
File: [critical] JavaScript should be compatible with other libraries than jQuery.

7.x-1.3 is of no interest here. If anyone is going to do anything then it'll be against 7.x-1.x-dev.

despite the fact the D6 v1.18 release is saying it fixed all coder module warnings, i just tested it today and a huge list is presented.

Coder (and PHP) evolve, too, and it's normal that new warnings and notices pop up over time.

Coding standards fixes are kind of fun. If you're willing to take them then I'll do more. If you prefer I start with Drupal 8 I'll do that too.
This patch contains mostly documentation improvements and does not include devel_generate.

Hey jacob.embree,

Your patch (#8) applied cleanly for me, but I'm still getting a lot of warnings from phpcs. I'm super new to Coder and phpcs, so forgive me if I'm all wrong about this. It looks like we're sniffing the code in different ways, so that may have something to do with it. I ran the following per these instructions:
phpcs --standard=Drupal --extensions='php,module,inc,install,test,profile,theme,css,info,txt,md' devel

Attached are two files with phpcs output. cr-output.devel.txt are the results without the patch and the other is after the patch was applied. devel_generate excluded.

zorya,
I don't know how the maintainers want to do it, so I figured on doing large chunks at time rather trying to fix every coder issue in one patch since there are so many. My patch has just documentation fixes. I'll probably keep plugging away at it, and if I ever manage to fix everything then I'll upload that patch.

Ah, gotcha. It's quite the hefty task. Makes sense to wait and see what the maintainers want.

I'm a bit on the fence here...

Generally, we want to spend most of the effort on the current version (read: D8).

We would like Devel to be a model of good code, but OTOH, sweeping changes mess up the version history. Sometimes we need to dig into the praise/blame history to understand why something is as it is, or even to just find the issue that produced a given line of code, and big time changes make this more difficult.

I've had a brief look at your #8 and, as you say, it's mostly documentation fixes, which is less of a problem for the version history. I lean towards committing #8 after taking a closer look, but I'd like to get Moshe's view, especially before you turn to work on the code.

Since you've written so many docblocks, you must know Devel better than we do by now. :-) As with any piece of software, I'm sure there are still bugs here and there--please don't hesitate to open issues if you find something that isn't right. Also, many of the rules are designed to avoid coding mistakes, and massaging the code to comply with the rules may definitely turn up hidden bugs.

Moshe?

I'm fine either way. Up to you, salvis. Thanks everyone.

Needs re-roll, then I'll commit it.

Thanks, jacob.embree!

Nice catch with the missing return.

I put back the commented dpm() calls that you removed in DNA. There's a lot of value in them, and if I ever had to do any debugging on that code again, I would need to waste time to re-develop them. I know that the Coding Standard says otherwise, but in this case I refuse to comply.