kcoworks [kcoworks]

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, pointing out what needs to be changed.

As per http://drupal.org/cvs-application/requirements, the motivation message should be expanded to contain more details about the features of the proposed module, and it should include also a comparison with the existing solutions.

If there is already a module for the same integration, why did not you contact the author, or open a feature request for a Drupal 6 version of the project?

Thanks for your reply.

subscribing

There was an error being generated due to this plugin invoke in emf with this module:

emf.sync.inc:193: $last_sync = emfplugin_invoke('unix_to_service_time', array(variable_get('emf_last_sync_subscriptions', 0)));

By adding this (without the open and closing php tags) to the end of emf_interspire.api.inc, the error is corrected:

function emf_interspire_api_unix_to_service_time($timestamp = 0) {
  if ($timestamp) {
    return date('Y-m-d H:i:s', $timestamp);
  }
  return 0;
}

Put in another vote for this having a project page and cvs access, it is a needed plugin and is modular and works at this point. Having an issue queue would allow more extension of this as well as assisting in keeping the issue queue clean for emf.

By the way, I'm open to assisting with being a co-maintainer if you desire to have one.

I am changing the status as per previous comment.

I am submitting two patches that correct the issues I saw working with it.

First, the hook_requirements in .install are looking for a value that doesn't exist:

 if (trim(variable_get('emf_interspire_api_key', '')) == '') {

I corrected this to look for the actual api items that are needed removing the error on the status page when configured correctly.

The second one refers to #10 up above.

Please upload the archive containing the corrected code.

Here ya go kiamlaluno.

Thank you.
I am sorry for bothering, but reviewing the code, and then reviewing two separate patches is not easy. :-)

I will review the code between 6 days. Feel free to update the code if you have other changes to make.

Will do - and no bother, I understand. It's especially tricky because of the requirement to have Interspire available for true testing - they only offer paid access to their software.

1. /**
    * Implementation of hook_install().
    */
   function emf_interspire_install() {
     $link = array('!link' => l(st('Administer > Site configuration > Interspire'), 'admin/settings/emf_interspire'));
     drupal_set_message(st("Interspire module settings are available under !link", $link));
   }
   
   /**
    * Implementation of hook_uninstall().
    */
   function emf_interspire_uninstall() {
     db_query("DELETE FROM {variable} WHERE name LIKE 'emf_interspire_%'");
     cache_clear_all('variables', 'cache');
   }
   

   t() is available to hook_install().
   The correct way to delete Drupal variables used by the module is to call .variable_del(), passing the name of the variables, which is well know when the code is complete. Selecting the variables to delete using just the module name can lead to delete variables of different modules (somebody could create a module named emf_interspire_extension.module, and the code would delete its variables too).

2.     '#description' => t('Your Interspire XML Path.'),
   

   Strings used in the user interface should be in sentence case.

Thank you for your contribution! I am going to update your account.
These 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 more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.

#2 confuses me, your example is in sentence case.

I have made the recommended changes from #17 and here is a new tar.gz of the module.

Sentence case is the case used in a sentence, where only the first word is written with the first letter in upper case; the string I reported should be written as Your Interspire XML path.. Interspire is the name of a company, and XML is an acronym; therefore, they are fine as they are written. path is a common name, and it is written Path only when it is at the beginning of the sentence.

Anyway, I approved kcoworks's CVS account already. There is no need to attach the code here, even if I appreciated you reported the fixed code. :-)

_{[Edited by kiamlaluno]}

I see the issue you mention, I'll be sure that is corrected as well, however I think there may be some confusion, I am not kcoworks, I just needed the functionality and stepped into help. I have sent a contact e-mail to kcoworks but not heard back from him regarding this module, I have no problem however, co-maintaining (or taking over if he is no longer interested) to help flesh the module out further and have it's own issue queue etc. On the EMF issue queue there are numerous other devs needing this functionality.

Thanks for clarifying for me either way.

or taking over if he is no longer interested

kcoworks's application has been approved less than 3 days ago; I don't think he is not longer interested. :-)

I didn't think so either, the thing about text as a communication form is tone of voice isn't maintained.

That said I have heard back from kcoworks and I am asking him to reply here, he is open to me as a co-maintainer.

If bonked doesn't apply for a CVS account, it's rather difficult to provide him one. :-)

Chicken and Egg ;-)

For co-maintaining existing projects:

• Create an issue in the module's issue queue requesting co-maintainership. You should only apply for a CVS account once the current maintainer has already accepted you as co-maintainer.
• Include a link to the issue requesting co-maintainership in the motivation message of your CVS application. Even if the current maintainer has asked you to become a co-maintainer, they still need to make a public record of this in the module's issue queue.

I'm assuming I need to wait until the project page exists at /project/emf_interspire. If not, could you point me in the correct location?

Until kcoworks doesn't create the project page, there is no way to add you as co-maintainer of the project he is creating. Anyway, that will be his task. :-)