[D7] Sirportly Integration

welcome,

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.

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

An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we please you to have a look at the report and try to fix them. Anyway, note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.
http://ventral.org/pareview/httpgitdrupalorgsandboxdanmatthews1728120git

It is highly recommend to get a review bonus so we can come back to your application sooner.

Would get back with manual review soon.

thanks.

Hi Dan,

You still have code on master, which should be removed so no-one mistakenly uses stale code, see http://drupal.org/node/1127732.

The branch should just be 7.x-1.x, not 7.x-1.x-dev

Some other comments here:

http://ventral.org/pareview/httpgitdrupalorgsandboxdanmatthews1728120git

I think there's some sensible workarounds for some of these, without having to change importedd library code, e.g. make sure you wrap the include_once 'libs/markdown.php'; in a check that the library isn't already defined (E.g. By another module).

Hi,

Still the code have lots of issue

The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags

./libs/class.php
./libs/markdown.php
./libs/markdown.php: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming

function Markdown($text) {
function identify_modifier_markdown() {
function smarty_modifier_markdown($text) {
./sirportly.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming

function blockAccessIfCredentialsNotSet() {
function sp_sort_ticket_array($a, $b) {
function __determine_all_sp_set() {
function cache_sirportly_metadata() {
function sp_recurseMenuItems($menu, $link_base = NULL) {
function dump($var) {

Note : Please review the code manually http://ventral.org/pareview/httpgitdrupalorgsandboxdanmatthews1728120git

Regards
Alok

I'm not sure the argument that "it is third-party so I am not responsible for adhering to coding standards" will pass.

Here is the official policy on third-party code: http://drupal.org/node/1001544

DO NOT include code from a non-Drupal project in the repository. If your module requires non-Drupal code, such as a third-party JavaScript/PHP library, provide a link to where the other code can be downloaded and instructions on how to install it.

All code should comply to the coding standards. Any patch that fails to do this will generally be treated as unfinished work.

From my own experience, it is much easier to comply.

Great.

What would be the chance of getting a testing account with sirportly to connect to? Could you set one up with some data in it so that we could help you test this module?

You can register for a free account here: http://sirportly.com/free or alternatively they offer a 30 day trail which I believe provides demo data.

Best of luck to you then.

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

Update git??

http://ventral.org/pareview/httpgitdrupalorgsandboxdanmatthews1728120git

There is still a master branch.

fix :)

#14 & #16

The master branch is no longer there (See below). I'd agree the other issues should get resolved though:

$ git clone http://git.drupal.org/sandbox/danmatthews/1728120.git sirportly_integration
$ cd sirportly_integration
$ git branch -a
* 7.x-1.x
remotes/origin/7.x-1.x
remotes/origin/HEAD -> origin/7.x-1.x

An automated review caught a few issues with your module:

http://ventral.org/pareview/httpgitdrupalorgsandboxdanmatthews1728120git...

You might want to try using that after you clean up the issues it found. It can be a great resource for the coding standards.

You've pasted your maintainer link in your project application. You might make it easier for others to test your module by adding the non-maintainer repository link.

After cloning your module, I followed the README and grabbed the php-wrapper from github.

This is what I have in my libraries:

/sites/all/libraries/sirportly/
- README.md
- class.php

I still get a warning though:
Please download and install the Sirportly-PHP library to use the sirportly module

I then visited /admin/config/services/sirportly and entered the api keys that you supplied above.
I got a 500 error:
PHP Fatal error: Class 'Sirportly' not found in /Applications/MAMP/htdocs/drupal-7.15/sites/all/modules/sirportly_integration/sirportly.module on line 634

Also:
Line 90 of sirportly.module

$items['admin/support'] = array(

Creates a very vague menu entry called "Support".

This should probably moved under admin/config/services/sirportly

What does the menu entry do? I can't actually look at it since the code is broken right now.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: http://drupal.org/node/1760016
Project 2: http://drupal.org/node/1865018

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

When installing the module it does not tell you what the library folder should be called.

Added version.