Skarabee

Skarabee offers an innovating online and offline software for professional real estate businesses in Belgium, France and the Netherlands.

This module is for real estate websites that use the Skarabee software to manage there publications and want to publish them on there own website.

This module will provide a new content type, create fields, import data, download images and insert and update publications on cron run.

Project page: http://drupal.org/sandbox/sboersma/1176520
Git: http://git.drupal.org/sandbox/sboersma/1176520.git
Drupal: 7

Comments

Comment #1

patrickd commented 28 May 2012 at 17:11

Status:

Needs review

» Needs work

welcome,

It seems like you´ve a misnamed branch (7.x-1.0) created in your repository, please have a look at the release naming conventions.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also create a README.txt that follows the guidelines for in-project documentation.

Your code documentation is very inconsistent (mostly missing function documentation); please make it follow the Coding Standards and the doxygen documentation formatting . You can use automated tools like drupalcs or http://ventral.org/pareview to help with this.

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

Comment #2

sboersma commented 16 June 2012 at 22:13

Status:

Needs work

» Needs review

Thanks Patrick for your quick review!

I've changed the branch to 7.x-1.x and created a tag called 7.x-1.0.
I've updated the project page following the guidelines.
Created a README.txt.
Removed all files from the "master" branch and added an README.txt to it.
I fixed all issues found by http://ventral.org/pareview (great tool).

I will checkout how I can review other projects, but I have to know if I do it right before I blame others.

Hope you will take another look at it.
Thanks!

Comment #3

cbeier commented 19 June 2012 at 10:17

Hi,

skarabee.module
In your menu declaration you should use the page callback 'drupal_get_form' for the settings form.

'page callback' => 'drupal_get_form',
 'page arguments' => array('skarabee_admin_settings'),

The skarabee_admin_settings_page() function in skarabee.admin.inc is then no longer needed.

You should use a better access permission for the settings form. E.g. 'administer skarabee'.

Maybe you should implement a validation function to check a proper wsdl, username and passwort input.

Regards, Christian

Comment #4

cbeier commented 19 June 2012 at 10:18

Status:

Needs review

» Needs work

Comment #5

sboersma commented 20 June 2012 at 18:35

Status:

Needs work

» Needs review

Thanks Christian!
I fixed the issues you mentioned in your review, except the validation, I have to look into this sometime.
Hope you will take another look at my project.

Thanks!
Sander

Comment #6

sboersma commented 3 July 2012 at 19:23

Hi Christian / Patrick,

I fixed all issues and implemented a validator on the WSDL.
Hope you can take another look at it.

Thanks!, Sander

Comment #7

sanchi.girotra commented 29 July 2012 at 09:51

Status:

Needs review

» Needs work

1. Please provide git link corresponding to non-maintainer like "http://git.drupal.org/sandbox/sboersma/1176520.git" in the issue summary.
2.Point your default branch to 7.x-1.x using project-page->edit->default branch.
3.Remove Master Branch using
git checkout 7.x-1.x (Current branch should not be master)
git branch -d master
git push origin :master

Manual Review:
1. In .install file use db_delete to delete all variables like "skarabee_%".
2. Declare types(skarabee_publication) in hook_field_schema(); rather then hook_install().

Comment #8

sboersma commented 9 September 2012 at 16:05

Status:

Needs work

» Needs review

Thanks Sanchi for your review.
I removed the master branch and changed the default branch.
Updated the install file for variables removal.

Only #2 in your manual review, to declare types in hook_field_schema() instead of hook_install().
When I look at the API it says "Define the Field API schema for a field structure.", so I don't think this is correct.

When I download the "examples" module, the "node_example" module, also defines the node type in the hook_install().

Can you, or someone else, explain why it should be in hook_field_schema()?

Comment #10

webdorado commented 19 September 2012 at 09:23

Status:

Needs review

» Needs work

We have installed the module and found the following bug.

After installing and enabling the module the content type is not added. The content type is added when you disable the module. When you enable the module again, the content type disappears.