This project is named "Controller Annotations" as this brings the neat feature from Symfony Framework to Drupal. As can be found in the description this project is hosted on Github and also tests (both functional and unit) are run with Travis on different combinations of Drupal and PHP (currently 20 combinations). Since I am a Symfony Framework developer I've used PSR-2 coding style rather than Drupal coding style. With this project I hope to bring communities closer to each other showing Drupal devs what nice goodies we have in Symfony Framework and on the other hand to show Symfony Framework devs how Drupal can be shaped into something they already know. Thanks in advance for checking this.
git clone --branch 8.x-1.x https://git.drupal.org/project/controller_annotations.git
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | Screenshot from controller_annotations.png | 161.84 KB | Gnanasampandan Velmurgan |
Comments
Comment #2
slootjes commentedComment #3
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgprojectcontroller_annotatio...
We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #4
slootjes commentedRemoved LICENSE file and added newlines to templates. The bot does not support PHP 7.1 which causes some error for "FooControllerNullableParameter.php" file which uses the nullable type. Other than that, as expected, it's about code style following PSR. As some files in Core also follow PSR I don't think this should be a problem.
Comment #5
slootjes commentedComment #6
slootjes commentedComment #7
arshadkhan35 commented@slootjes
1. pareview.sh gives some errors.
https://pareview.sh/node/2654
Please fix these error.
Comment #8
slootjes commentedarshadkhan35: I only see 2 things: the bot doesn't understand PHP 7.1 which is used in a test and the code style. If we can ignore the code style (for reasons described earlier) it should be good, no? :)
Comment #9
slootjes commentedComment #10
Gnanasampandan Velmurgan commentedI found some issue on your module on PAreview. Plesae fix it.
https://pareview.sh/node/2654
Comment #11
slootjes commentedPlease read the comments before posting the same link for the 4th time, thank you.
Comment #12
slootjes commentedBump
Comment #13
sutharsan commentedI'm having a look at the module.
Comment #14
sutharsan commentedThis is a large amount of code to review. I did check various parts for architecture and security, but certainly not all the code.
The test coverage looks good. I did not measure the coverage, but the number of available tests is very nice. I can only wish other modules do this too. I checked that the route access test covers many scenario's.
I have some experience with @slootjes and I trust his experience and expect him to write well thought-out code.
Regarding the code style. I do encourage the discussion, but there has been extensive debates before where it has been concluded that it is not worth changing to PSR-2 style. See #2645010: Switch to PSR-2 as of Drupal 9, #2713641: PSR-2: Opening braces and #2918624: Change indent from 2 spaces to 4, to comply with PSR2. Based on this, I vote not to allow PSR-2 code style in contrib modules too.
Comment #15
avpadernoTo the reviewers: Please set the priority to Normal after reviewing the project.
Comment #16
Piyush_Rai commentedThere are many errors and warnings in this module. Please use command PHPCBF to resolve instantly these errors.
Please make sure that the lines could not be exceed more than 80 characters in README.md.
Comment #17
Piyush_Rai commentedComment #18
slootjes commentedComment #19
slootjes commentedI will leave the PSR-2 code style as is, if that's a blocker for getting the security badge so be it. Thanks everyone for their time :)