[D7] Merlin

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxchurel2160815git

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.

Your project page says:

Requirements
A behat environment with a javascript browser (http://behat.org/).

However, when I open the README.txt, it tells me:

4. Get Selenium (http://docs.seleniumhq.org/download/).
On a stand alone version
(selenium-release.storage.googleapis.com/2.42/selenium-server-standalone-2.42.2.jar)

And when I try to run composer, it tells me my PHP version is too old!

And to top it all, the repo comes with some binary files in the directory "ods-templates" and I can't find it documented anywhere what they are and how they're supposed to be used, or what I need to install to make use of them. (OK, I know they're spreadsheets and that having OpenOffice.org is probably a requirement as well, but these things should be documented on the project page).

There is nothing to brighten the day of a reviewer to get these little surprises after having set up the environment, rather than up front.

I think, as a minimum, the project page need to list all the necessary bits and pieces that need to be in place before one is able to make use of the module. I feel I just wasted my time on this one.

Also, avoid unexplained jargon such as "Gherkin suite" and "Gherkins scenarios". Yes, I know how to google, but I don't think I should have to do such things in order to learn how to use a contributed Drupal module.

Please take a moment to make your project page follow tips for a great project page.

Additional points:

Automatic review

PAReview had some minor complaints: link to review.
Please review the output of PAreview. Make a note in the issue summary if you think they're false positives.

Master Branch

It appears your sandbox is set up with the depreciated "master" branch in as default (even if the 7.x-1.x branch exists). You should really be have a version specific branch as the default. (PAReview thinks you also should remove the "Master" branch. I don't think that is necessary, but the default branch should never be "Master"). Please see the documentation about release naming conventions.

@gisle
you may have to update your PHP, i believe behat needs 5.4
I'm adding a patch to specify that in merlin.

of course, the documentation (or faults/lack of) of behat or seleniumhq is an issue to those tools.

I think an important thing to keep in mind is the project your testing may only need to be running in 5.2 or 5.3 and then might want a different host env to run your project.

what Merlin does

• it builds Content-types with fields into Features and then separately, tests ready for Behat.

----

The part of merlin that builds the initial tests and features it-self would only need to run in 5.3. Some more development and/or documenation will evolve from this.

which that my patch should only specify 5.3, not 5.4

manual review:

1. merlin_menu(): why the permission 'access content overview'? For importing stuff that does not make sense? Also this looks like an unexpected feature for users that have the permission, one could even argue a security problem?
2. merlin_import_form_submit(): doc block is wrong, this is not a hook. See https://www.drupal.org/coding-standards/docs#forms
3. merlin_import_form_submit(): $something and if there is something to do should be determined in the validation callback of the form?
4. merlin_handle_ods(): consider using drupal_mkdir() instead instead of mkdir() to respect Drupal's file creation chmod.
5. merlin_build_site_creation(): file_put_contents() is nice and does not require you to use all the file handling functions.
6. merlin_help(): doc block is wrong, see https://www.drupal.org/coding-standards/docs#hookimpl . Also elsewhere, please check all your hooks.
7. I think the README.txt should mention that this should never be enabled on a production site, right? This is for development sites and building sites, right?
8. merlin.ods.inc: did you modify the code of this library? I think LGPLv3 is not compatible with GPLv2+ used on drupal.org, so I think this file must be removed. If you did not have to modify it then you can just tell users where to download it. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.
9. merlin_parseOds(): shell_exec()? Really? This will fail on some PHP installations that don't allow shell execution. Why can't you use ZipArchive or something similar again?

So the library thing is a blocker right now, please clarify.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.