Formstack Online Forms

Hello,

Ventral (ventral.org) reports some coding standards issues - http://ventral.org/pareview/httpgitdrupalorgsandboxformstack1592982git

It looks a lot, but you just need to get used to them (I went through it all too :) ) - you can solve a lot of them with a find and replace call. Also keep using ventral when you update GIT and you'll get through them in no time.

Also 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.

Some other things :-

README.txt is missing, see the guidelines for in-project documentation.
Remove "version" from the info file, it will be added by drupal.org packaging automatically.
The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
./formstack.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
function my_module_node_post_render($sth) { looks like you left an example module in :)
PHP Parse error: syntax error, unexpected '/', expecting T_FUNCTION in ./formstack.api.php on line 37
Errors parsing ./formstack.api.php

Hope this helps - just drupal standard stuff.

I think the parse error means it needs work? But will leave on needs review. I'd try and fix the parse error ASAP though so people can test it.

Hope this helps

Pat

these are enough major issues for changing needs work ;)

Hi there,

Automated review looks all clear now apart from the one message saying you are still working in the master branch. Have a look at Moving from a master to a major version branch - basically you'll need to do:

git checkout master
git checkout -b 7.x-1.x
git push origin 7.x-1.x

You'll then want to remove the code from the master and leave README.txt there telling anyone who checks the wrong branch out to move to one of the major revision branches.

A further look through the module has flagged up the following thoughts for me:

• formstack.api.php - I'm pretty sure api.php files are used for defining hooks within the module, I think it might be better if this was a .inc file to avoid clashing with naming conventions.
• In the module file itself, when you use a hook the main comment line should read "Implements hook_X(). See for example line 10 which should read Implements hook_menu().
• Output copy on line 128 should be wrapped in t() function
• You have a few references to the dreaded 'und' in _formstack_node_form_submit(), it would probably be wiser to set, say $lang, to the node language and reference e.g.
  $node->body[$lang][0]['value']
  instead of
  $node->body['und'][0]['value']
• In function _formstack_create_embed_code() would you be able to use drupal_add_js() instead of returning the full embed markup?

Cheers!

Oops forgot to change status back

Sorry, but there are currently about 100 other applications, some of them already waiting for months without review and just a handful of active reviewers. The only way to speed up the process is by reviewing other applications.
Have a look at the review bonus program: https://drupal.org/node/1410826

If you use the given git clone command on your project page, you still will get the master branch instead of the new branch. Have you changed the default branch from master to 7.x-1.x under edit/default-branch on your project page ?
Also the git command given in this issue summary will not work, because we can't authenticate as Formstack user, this one should work:
git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/Formstack/1592982.git formstack_online_forms
I noticed that you can't insert multiple Formstack forms into one node via the GUI. It will always replace the existing form ( you can insert multiple form tags manually if you know the corresponding id ). Also I think it would be nice, if your form tags would be more human readable and also include the form title along the id.
Maybe adding a help page for your module and writing a test file would be a good thing too.

All issues explained in #4 seems to be resolved without the last one. Is it possible for you to use drupal_add_js()?

Everything else seems to be fine.

Rather than overriding $node->body in _formstack_node_form_submit it would probably be better to attach a Formstack form to a field. I need this for a project. I'll update in the sandbox issue queue if I get anywhere.

After discussing the topic on IRC in #drupal-contribute, since this issue has not had any progress from the poster in over 6 months, I've gone ahead and created a Formstack project on drupal.org. http://drupal.org/project/formstack.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

git clone branch changed from master to 7.x-1.x