I love this project!! I love that we are using standard tools others in the wider PHP community are using to deal with this problem. I love that we have IDE integration for coding standards. I love that there are active maintainers. Yay!!

However, we also have http://drupal.org/project/coder which is our own home-grown thing, not as actively maintained. But one thing Coder has going for it is it is now integrated with testbot: http://qa.drupal.org/8.x-status#tabset-tab-4

Is integration with Project Issue File Review something on the table? I'd love to point people rolling pedantic patches like #1373582: Trailing whitespace at instead putting efforts behind automating these checks, but can't in good conscience recommend they do it here unless we use it on d.o.

CommentFileSizeAuthor
#2 pifr_drupalcs.zip7.22 KBdas-peter
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

das-peter’s picture

das-peter CreditAttribution: das-peter commented

I just took a look in PIFR and it seems not to be that hard to create such a plugin. The real "challenge" seems to be the proper configuration of the environment.
Since drupalcs depends on PEAR it isn't a drupal installation as usual.
I'll dig deeper and maybe I can come up with a proof of concept.

das-peter’s picture

das-peter CreditAttribution: das-peter commented
FileSize
pifr_drupalcs.zip7.22 KB

I've just tried to create a PIFR plugin.
It's a quick n dirty copy of pifr_coder.
I tried to get PIFR run on my environment (Windows ;) ) - but I couldn't manage it that fast.
However I did my best to change the necessary stuff.

The plugin works similar to the coder plugin - it passes every single file to the PHP_CodeSniffer.
Then pifr_client_review_pifr_drupalcs::get_result() parses the output of the sniffer to create the necessary messages.

There also new configuration settings in /admin/pifr/configuration to configure pifr_drupalcs.

Would be great if someone familiar with PIFR could check if the approach isn't totally wrong.
Meanwhile I'll try to get PIFR running on my system to check if the plugin really works ;)

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented

@webchick: I talked to jthorson about this and we agreed that drupalcs will need a little more time to mature. There are some issues about big changes in drupalcs (moving code around, renaming stuff) that should be done before we even think of pifr integration.

@das-peter: I suggest to put your code into a sandbox. And we should notify jthorson about this, as he is most familiar with the pifr integration (he did it for coder).

das-peter’s picture

das-peter CreditAttribution: das-peter commented

@klausi: Thanks four you feedback.

Sandbox created: http://drupal.org/sandbox/daspeter/1383456

I don't think there's any blocker that would stop creating a PIFR integration at this moment. Because it's less about integrating drupalcs and more about creating / ensure an environment to run PHP_CodeSniffer.

The pifr plugin itself has nothing drupalcs specific - you could run every PHP_CodeSniffer standard you want to.
The plugin only allows to define the path to PEAR (for the case it's not already in include_path) and you can set a name or a path to the standard to use.
On execution the plugin configures the PHP_CodeSniffer, starts the processing and parses the result.

This means this integration should work with drupalcs as long as drupalcs doesn't break any PHP_CodeSniffer stuff.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Holy crap, AWESOME!!! :D Way to go, das-peter!!

And cool, glad jthorson was already brought into this discussion, and certainly don't want to undermine him in my raw enthusiasm. :D I agree that Coder is further along in terms of its stability, and also its knowledge of Drupal's coding standards (despite numerous false-positives). But I agree with das-peter that there's tremendous value in our testing tools integrating with standard, external review tools, so am THRILLED to see how quickly a starter extension came together! Seriously, awesome stuff!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Active » Closed (won't fix)

Drupal Code Sniffer has been merged into Coder 7.x-2.x. Please move this issue to the Coder queue and reopen it if the problem still exists.