IMCE plupload

Hello,

Ventral (ventral.org) reports the following coding standards issues - http://ventral.org/pareview/httpgitdrupalorgsandboxkingdutch1592038git

You might want to download Drupal Code Sniffer, and check the Drupal coding standards.

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

• Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
  
  

  <br>
  ./imce_plupload.info:   ASCII text, with CRLF line terminators<br>
  imce_plupload.info<br>
  imce_plupload.module<br>
  

Give those a quick try - ventral is really handy for testing the standards issues, but it does miss the odd thing that Code Sniffer finds.

The list does look big but you can rip through them quickly enough (once you get used to them)

I develop using XAMPP locally on my pc - and use DCS on that? It helps.

Also try the Code Review module (coder I think is its name) - which does a similar thing.

no other output? Looks like it might be ok then - what did you type on the command line?

When I am in the module I want to test I use

c:\xampp\php\phpcs --standard=drupal --extensions=php,module,inc,install,test,profile,theme .

Where c:\xampp\php\phpcs is the path to the phpcs file.

Hope this helps.

Checked your module.

Manual testing:
-Works. Installs according to readme. Dependencies ok.

Automated review:
-Small coding style error

FILE: ...publicatiedatabank/sites/all/modules/imce_plupload/imce_plupload.module
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 30 | ERROR | Function comment short description must end with a full stop
 77 | ERROR | Function comment short description must be on a single line
---------------------------------------------------------------------------

Code review:
-Clear code. Nice simple module, which does exactly what's promised.
-2 nitpicks
// Core bug #54223.
Replace with summary of bug, since bug is closed (won't fix)

Replace $tmp variable with better name.

But seems very good.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Kingdutch, thanks for contributing this work. It looks like a very useful addition.

IMHO, #1591410: Insert multiple files is a much better integration path for this code than in a separate project, and it looks like project the maintainer is active, so reviews there would be of much higher impact. If the application review process were structured a bit differently, I'd be happy to vet this based on the code quality, but I'll just keep an eye out for your next project application. Otherwise, feel free to ping me if you want a review or other feedback. Thanks again! :)

I wouldn't agree with the previous comment.

This project application is similar to the IMCE SWFUpload project in that it uses the third-party multiple file upload library through the separate integration project (already published on drupal.org).

The #1591410: Insert multiple files issue doesn't use any third-party library to handle multiple file upload. The thing is that these third-party libraries are really powerful when you face the huge file uploads - like several gigabytes as they allow for splitting the huge files into separate small parts and upload them in parts (it is called "file chunking"). Plupload is really great library, and the project from this project application uses the plupload integration module, so please-please approve this project.

Thanks
AndyB

Beside the possibility of understanding this module as duplicate or not there is not enough code to approve.
The limits established are 120 lines of php code and at least 5 functions.
For more details you can read this: http://groups.drupal.org/node/195848

changing status

But we can promote single projects if we want.

It looks like this sandbox is having a status ping ball :-)
The code seems good... If there is no duplication problem as mentioned at http://drupal.org/node/1592084#comment-6275826 then I guess it should be single promoted.

Beside that it would be good to have a more detailed description (http://drupal.org/sandbox/Kingdutch/1592038). You can read http://drupal.org/node/997024 for more info.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732

manual review:

• project page is too short, see http://drupal.org/node/997024

Not sure about whether this is a duplication or not, I give you the benefit of the doubt ...

Thanks for your contribution, Kingdutch!

I promoted this project for you: http://drupal.org/project/imce_plupload

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.