Hi guys,

Thanks a lot for this amazing script, it is incredibly useful to developers and code reviewers.

While working on some project application reviews, I was brought to working with the online service of the script available at ventral.org:

There is an online version of pareview.sh on ventral.org.

(Quoted from module's page)

As I was reviewing one of the applications (See #1848868: Range field), a user (Taran2l) was complaining about some coding standard issues that were prompted on code that had been directly copied from the drupal-7.x core, Number module, number.module file.
See comment at #1848868-8: Range field:

Regarding #1848868-4: Range field, I do not know the procedure either, but I think if this code exists in core, it can exist in contribute module.

Which gave me the idea to try to verify this information on PAReview online (ventral.org), whether the automated review of the drupal core GIT 7.x branch could be generated, and I submitted the following value in the field URL to git repository [optional: branch name]:
http://git.drupal.org/project/drupal.git 7.x

Unfortunately, it didn't work and prompted an error message:

Sorry! There was an error during the process.

I would maybe assume it is because this GIT branch is too large to be processed and therefore prompts some kind of timeout error (or maximum execution instructions, time or something like that).... but that's just a supposition.

I then tried submitting the form again simply with: http://git.drupal.org/project/drupal.git, but the result was the same.

Bringing me to point of this issue:

I was wondering if these tests were also taken into account for the Drupal Core GIT Branches developments.
Would there be any way to get an online review of a Drupal Core GIT Branch?
Would there be any existing URL that I wouldn't be aware of?

Would anyone please be able to kindly advise if there would be any way to get an automated review of a Drupal Core GIT Branch (For example, 7.x)?
Would you happen to have any experience setting up any environment (for example, the PAReview script) and running it on a Drupal GIT branch (For example, 7.x)?

I would greatly appreciate if you could give me some guidance or your feedback on this and let me know if I missed something in the parameters for the PAReview form submission or overlooked anything.

Feel free to let me know if anything in this summary is unclear or if you would have any questions on any aspects of this ticket, I would surely be glad to provide more information.

Any feedback and comments would be highly appreciated.
Thanks in advance. Cheers!

Comments

klausi’s picture

Status: Active » Fixed

pareview.sh is just a wrapper around coder sniffer: http://drupal.org/project/coder

So you will want to install coder sniffer and run parts of drupal core through it.

Drupal core is not perfect, there is a meta issue to coordinate the efforts: #1518116: [meta] Make Core pass Coder Review.

DYdave’s picture

Hi klausi,

Thanks a lot for your very prompt, clear and helpful answer.

pareview.sh is just a wrapper around coder sniffer: http://drupal.org/project/coder

So from what I understood here, I'm not sure of the exact details, but it seems the code analysis and review is actually provided by the Coder module and the server side stuffs, for automated review based on GIT branch (pretty much what is used on ventral.org) is provided by pareview.sh.
So the contents of the code review/analysis in itself is actually provided by Coder.

Additional note:
I actually created this ticket after I tried running a Coder review of Core modules on a local environment, but the errors I was looking for in the number.module were not being prompted/showing, so that's why I thought I couldn't find any equivalent to the online version reviews, to be installed locally.

I still haven't figured out why some errors, such as:

16 | ERROR | If the line declaring an array spans longer than 80 characters,
 | | each element should be broken into its own line
23 | ERROR | If the line declaring an array spans longer than 80 characters,
 | | each element should be broken into its own line

wouldn't display, when I review the number.module with Coder.
I guess there are some additional configuration parameters that I must have missed.
But I guess this is an issue that I will be bringing elsewhere, most likely in the Coder module's tracker.

 

FYI, your helpful answer allowed me to post back at: #1848868-9: Range field.

Drupal core is not perfect, there is a meta issue to coordinate the efforts: #1518116: [meta] Make Core pass Coder Review.

By following the link you indicated, I was able to find a suggested patch for the number.module (#1533232-2: Make field module pass Coder Review), fixing coding standards issues, to try to convince a user on a project application #1848868: Range field.
(I thought you might be interested as well, since it's related with Project Applications, so feel free to take a look and please let me know if you would have any feedback or comments as well)

Feel free to let me know if you would have any questions, comments or concerns on any aspects of this answer, I would surely be happy to provide more information.

Thanks again for all your feedbacks, comments and help.
Cheers!

DYdave’s picture

ok, I think I got previous point figured out.

It seems that Drupal Code Sniffer would be the one to provide the reviews for the errors mentioned above:

I still haven't figured out why some errors, such as:

16 | ERROR | If the line declaring an array spans longer than 80 characters,
 | | each element should be broken into its own line
23 | ERROR | If the line declaring an array spans longer than 80 characters,
 | | each element should be broken into its own line

wouldn't display, when I review the number.module with Coder

which seems to be integrated with the Coder module only in the 2.x Branch.
It also seems to require additional PHP configuration settings, with specific modules, that I didn't setup before running the tests.
Since I have been using coder-7.x-1.x branch, which doesn't integrate Drupal Code Sniffer, then that's why the errors wouldn't display.

Which pretty much settles this issue and answered all my questions.

Feel free to let me know if you would have any questions, comments or concerns on any aspects of this answer, I would surely be happy to provide more information.

Thanks again for all your feedbacks, comments and help.
Cheers!

Status: Fixed » Closed (fixed)

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