[D7] ooe

Concerning preview review. It claims:

Remove "version" from the ./ooe.info file, it will be added by drupal.org packaging automatically.

The ooe.info file does have version in there, but commented out, including comments about the version tag (this is deliberate as it is supposed to be an educational module). From http://cgit.drupalcode.org/sandbox-webel-2120905/tree/ooe.info with quotes from Drupal.org documentation:

; version = "7.x-1.x"
; (Discouraged)
; 'The version string will be added by drupal.org when a release is created and a tarball packaged. 
; However, if your module is not being hosted on the drupal.org infrastructure, 
; you can give your module whatever version string makes sense (eg. see Release naming conventions).
;
; https://drupal.org/node/1015226 Release naming conventions
;
; Users getting their modules directly from git will not have a version string, 
; since the .info files checked into git do not define a version. 
; These users are encouraged to use the git deploy module to provide accurate version strings 
; for the admin/build/modules page for modules in directories checked out directly from git.'

Paraview should be improved to only report on this only if 'version' is explicitly used.

Concerning whitespace report. The Paraview reports on whitespace at the end of lines in some comment docblocks, but Coder does not. For example, on the file lib/Drupal/ooe/Menu/DefaultMenuItem.php it reports on whitespace 100s of times, but when run through Coder 7.x-2.0 it detects no whitespace.

I am using NetBeans IDE and it seems to insert a whitespace after any empty docblock '* ' line. This is very hard to remove, and also completely harmless anyway.

[EDIT: external links to assist with using NetBeans with Coder/PAReview Coding Standards:

- TIP: NetBeans: removing trailing whitespace (to meet the Drupal Coding Standards and pass Coder/PHP_CodeSniffer)

- TIP: PHPCSMD: a plugin for PHP Code Sniffer compatible with NetBeans8 and the Drupal Coding Standards from Coder

]

Please note that I can't, for this special UML-friendly project. under any circumstances simply apply PHPCBF to "clean up" this code, it would completely corrupt the synchronisation of the code w.r.t. to the UML model. In any case, as explained above, I strongly contest some of the current Coder rules for OO projects, and PHPCBF would destroy my more systematic and OO friendly coding style.

Paraview is reporting on type hinting of parameters to operations/methods. For example, in the following from MenuItemSet:

  /**
   * Adds an array of child menu item to this.
   * 
   * @param IMenuItem[] $menuItems
   *   The child menu items to add.
   * 
   * @return IMenuItemSet
   *   This.
   */
  public function addMenuItems(array $menuItems) { ..

The paraview reports:
51 | ERROR | [ ] Expected type hint "IMenuItem[]"; found "array" for $menuItems

It seems to be reporting on the parameter with type hint '(array $menuItems)'.

There is no mention of this kind of type hinting under http://php.net/manual/en/language.oop5.typehinting.php (search for '[]').

And Coder does not seem to report on this either.

Coder 7.x-2.0 is not catching whitespace at end of docblock parameter declaration. Here I have appended a whitespace as an underscore after $menuItems

  /**
   * Adds an array of child menu items.
   * 
   * @param IMenuItem[] $menuItems_
   *   May be null or empty (will be ignored) !
   * 
   * @return IMenuItemSet
   *   This.
   */
  public function addMenuItems(array $menuItems) {

The Paraview is reporting on this (copiously as it turns out because NetBeans leaves whitespaces after such on hitting RETURN) as:

45 | ERROR | [x] Whitespace found at end of line
45 | ERROR | [ ] Parameter comment must be on the next line
46 | ERROR | [ ] Parameter comment must end with a full stop

Paraview is falsely reporting that there are the incorrect number of lines before parameter docs when @link @endlink is used. From IMenuItem.php:

  /**
   * Sets (from hook_menu()): 'The untranslated description of the menu item'.
   *
   * Visit also:
   * @link https://www.drupal.org/node/323101 Strings at well-known places: built-in menus, .. @endlink
   *_
   * @param string $description 
   *   'The untranslated description of the menu item.'
   *  
   * @return IMenuItem  
   *   This.
   */
  public function setDescription($description);

Paraview gives:
58 | ERROR | [ ] There must be exactly one blank line before the tags in a doc comment

[EDIT: Or it might be because of a whitespace char on the line above, and nothing to do with @link @endlink, I've marked it with an underscore above.]

Coder 7.x-2.0 does not trip on this.

Update on this application from the developer, and response to deactivation of this module submission by the robot.

The Object Oriented Examples (OOE) project for Drupal7 is very active, and is the result of a very intensive and extended period of development by the author, although until project posting approval has been received, all further development is on hold.

Reviewing this project according to the Drupal module development standards may require more human attention than most other modules.

The module is for academic/educational purposes only. The primary, and significant, purpose of the module is to demonstrate a PHP coding recipe for truly object-oriented Drupal modules that is amenable to reverse engineering to graphical Unified Modeling Language (UML), as explained at http://drupal7demo.webel.com.au/ooe and demonstrated at this unique UML diagrams gallery for the module. This special aim, a UML-friendly Drupal7 module, requires a special adaptations of the Drupal coding standards, as explained in detail at the demo web site.

Because of the special aims of this module, graphical UML-friendliness, it is impossible for a robot to interpret the module's PHP code, coding style, etc. as it would for another module.

Finally, as explained in the original application, you can't simply download and run this module against Drupal7 (yet), because integration of its object-oriented controllers requires 2 minor tweaks to Drupal7 core, as explained at the demo web site at OOE: special tweaks to Drupal7 core's form.inc required to support object-oriented form controllers (where it is explained also how one can easily make those core tweaks in PHP code without any adverse side-effects). You can see the module functioning live (with those Drupal7 core tweaks in place) at http://drupal7demo.webel.com.au/module/ooe.

I appreciate that these matters perhaps require more time investment from the reviewers than most other modules, but I contend that this OOE module, which is perhaps the first ever Drupal7 module to offer completely consistent graphical Unified Modeling Language (UML), makes an important contribution to Drupal that warrants very detailed examination.

From Git coding standards fix branch commit (since merged onto 7.x-1.x):

[FIX/PAReview_20140324 f024a30] 
A major coding standards sweep was performed through every PHP file, as well as .info and .install. This sweep reduces the errors reported by PAReview.sh by about 80%, but because of the specific UML-friendlier PHP coding style used here, and because of disagreements with the Drupal Coding Standards for OO, and disagreements with inline comment rules, this module does not and will likely never 100% correspond with the Drupal Coding Standards, and this is deliberate.

There are also some bugs in Coder (still present in 8.x-2.1). Placing @link at the beginning of a docblock line can cause this error:

Parameter tags must be defined first in a doc comment

Placing a single word before a @link prevents this, but then often causes the line with the @link to extend over 80 characters

- #2463707: @link @endlink starting 2nd line causes 'Parameter tags must be defined first in a doc comment' in methods

- #2463401: @link @endlink starting 2nd line causes 'Doc comment long description must end with a full stop'

- #2463397: @link @endlink alone on 2nd line gives 'There must be exactly one blank line before the tags in a doc comment'

See also:

- #2464123: Remove the requirement that no blank line follow an inline comment

- #2463425: Permit exclamation mark ! punctuation at end of parameter comment

- #2463633: Interface methods without explicit public visibility modifier should not trigger 'ERROR Visibility must be declared on method'

- #2306409: Inline comments: not all cases should require a full stop, exclamation mark, or question

IMPORTANT UPDATE: this module requires some minor, harmless, tweaks to Drupal7 core's includes/forms.inc and includes/ajax.inc in order to support static class methods for object-oriented controllers. A patch for Drupal-7.x-dev as of 2015-04-04 (Drupal-7.36+) has now been provided for this, as explained at https://www.drupal.org/node/2166371#comment-9792067.

This is not an attempt to HACK Drupal core ! On the contrary, it is hoped that the community will review and support these harmless changes - made only to support static class method callbacks in Drupal7 - so that they will be included in Drupal-7.37.

To make it easier for the Drupal developer community to review and use this experimental, educational, OO tutorial module, I have now provided 2 easy ways to download and test it, with detailed instructions at these external links:

- Download: a Drupal-7.x-dev patch to support the OO form controllers of the OOE tutorial module

- Download: OOE bundle with adapted Drupal-7.36 and installation instructions

Having waited since Aug 2014 for ongoing human attention I am now moving this to status 'critical' in accordance with the Application Review Timelines.

Update concerning coding standards and Coder 8.x-2.x. From commit message, merged onto ooe-7.x-1.x:

[FIX/Coder_8x2xdev_20150405 23d4860] 
Code Sniffer sweep against Coder 8.x-2.x-dev. Any remaining departures from Coder rules are either due to bugs reported under Coder issues, or they are deliberate departures due mainly to disagreements with the Drupal Coding Standards as applied to OO coding, as explained under http://drupal7demo.webel.com.au/node/1759, or due to disagreements on comment standard recommendations. There are also some deliberate minor departues from Coder rules preserved for sake of demonstration for Coder and Drupal Coding Standards issue reporting and tracking.

@himmatbhatia wrote:

1) Git Clone url is improper, It should be this. Your git clone command should be like below should have directory name. New user will get confused.

git clone --branch 7.x-1.x git://git.drupal.org/sandbox/webel/2120905.git webel

Why ? The Apply for permission to create full projects page says:

You can find the correct git clone command for your sandbox by clicking on the Version control tab, removing the checkbox in front of "Maintainer", and clicking Show. You can then copy-paste the git clone command from the codeblock below "Setting up repository for the first time".

If I do this it gives:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/webel/2120905.git

@Fabianx

Your suggestions are most welcome and very constructive.

> Do not submit huge projects like this as a project application, ..

Just to be clear, the reason that it is huge is that is trying to demonstrate no less that an alternative possible future for Drupal, for those who are interested in pure OO and in UML-friendlier PHP, as well as model-driven development for PHP-driven Drupal. In fact, the module only has a few small active demonstration packages and examples, but ultimately the idea is to provide a pure OO bridge for Drupal7 and if needed (because it is not pure OO and is not UML friendly) for D8,

I have another module called Flag Plus that might fit the bill, it adds a couple of features to the Flag module that I use regularly.

But it (like this OOE module) does not - because I refuse to do it - use 2 of the current coding standards recommendations:

- #2474561: Coding standard: inconsistent requirement that arguments to setters methods should be $lower_case when private variable and setter methods are camelCase() (because using $lower_case as the name of a setter method argument foro a private variable that is $camelCase is one of the stupidest things I have seen in over 3 decades of programming).

- #2304937: Please remove this requirement: 'WARNING | Interface names should always have the suffix "Interface"' (because with any modern IDE, and when using "design by contract" and "design against and interface" - and especially with UML - is completely unnecessary, in the same way that sticking 'Class' in every Class name is, and I refuse to do it, because I find it utterly horrible for the reasons explained there).

None of my modules, all of which demonstrate UML with PHP, will ever use such ghastly coding standards.

In any case, It is not clear to me that one could make perhaps also submit a 2nd Project Applications module; I have hesitated to withdraw this OOE one, having already waited one year.

> please provide ooe_block or ooe_render or whatever as project application target and I can guarantee that it will go in way faster than this code base.

This is a good suggestion, I had not thought of that.

The problem however is that until the matter of D7 forms.inc excluding object-oriented form controllers, the whole ecosystem in which the OOE Packages live can't be easily tested by people. I will revisit this idea of yours in mid August to see whether I can isolate some part that can be tested as a standalone module.

Or there is another way

Of course, Drupal.org could finally get rid of this incredibly painful Project Applications process that almost no other open source effort uses, so that people can simply contribute instead of having their time massively wasted: As argued by someone else very well here Drupal is still a gated community.

As far as I am concerned, this entire review process is nothing but a painful imposition that should be chucked in the bin.

So we can give. Easily. Without fuss. Without this pain.

Because the Project Applications process does not work; it just turns good people off

Webel

@Fabianx

I do not suggest that you do anything at this point. After all I have taken the time to review the project in depth and put this issue to RTBC

Many, many, thanks.

I appreciate that your other feedback is very constructively intended. To be clear (for other potential readers):

you cannot expect someone to read thousands of lines of code.

I don't. I don't even want a Drupal Project Applications process, or to inflict it on other volunteers. I did not ever ask for it. I just want to promote my module out of the sandbox.

Certain Drupal.org members - most of whom already have "the keys to the kingdom" - do (expect others to spent time reading such code).

In this case you made your life much more difficult than it needed to be ...

No I did not. The existence of the Drupal project applications process did. All I did was develop a module according to its own nature.

Same point applies: I don't want a Drupal Project Applications process, or to inflict it on other volunteers. I just want to promote my module out of the sandbox. Certain Drupal.org people with the keys to the kingdom made it harder that it needed to be. Much, much harder.

And I should not have to deliberately develop a module other than this, just because of a process that is clearly not working to the advantage of anybody anyway.

Also, your sensible suggestion that I could for example isolate part of my large project as a distinct module has in my case the extra burden that I have an existing set of synched UML diagrams for educational purposes (not something that everybody else has to consider), and I would have to redo many of them if I split my OOE project into separate smaller modules, which would therefore be even more work for me.

It would (if indeed now still needed) be easier for me to (after one year's waiting) to withdraw this one, and post instead a smaller module like my Flag Plus sandbox project. Which would be very sad indeed, to see no benefit from my effort to date regarding this Project Applications process.

Webel

@cweagans

Thanks, this means a huge amount to me, having waited so long, and having put so much work into this comprehensive experimental educational OO module.

I will take advantage of this to promote 2 modules from the sandbox in early August (am busy with a contract right now and am also having some Git problems I need to address first).