DongIT [dongit]

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

As per requirements, the motivation message should be expanded to contain more features of the proposed project. For themes, it should include also a screenshot of the theme, and (when possible) a link to a working demo site; for modules, it should include also a comparison with the existing solutions.

• The points reported in this review are not in order of importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

1. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see Drupal variables, global variables, constants, and functions defined from the module should be named; how the code should be formatted.

2.   drupal_add_js('var module_path = "' . base_path() . $module_path . '";', 'inline');
   

   drupal_add_js() can be used to pass settings to the JavaScript code.

3.   if (count(smids) == 0) {
       return 'No sitemaps found.';
     }
   
   

   Strings used in the user interface should be translated.

4. The module depends from a PHP5 function, but it doesn't declare its dependency from that PHP version (Drupal 6 is still compatible with PHP4).
5. As per requirements, the description of the module given in the motivation should be few paragraphs instead of few sentences. Even in the case few paragraphs should be too much, the description given for the module is too few.
6. Did you ask to the current maintainer of XML sitemap if he was interested in adding the feature implemented in the proposed module in XML sitemap?

Contacting me via contact form when I don't know who you are though tends to usually get ignored. Had I seen this in the XML sitemap issue queue I would have gotten a quicker response that this doesn't really belong in the XML sitemap module code.

@Dave Reid: Thank you for your reply.
I should have asked if a feature report was opened in the XML sitemap issue queue, as that is the proper place to ask for feature requests.

Hi,

If "This module is an add-on to the XML sitemap module", The first step, which you say you have tried, is definitely that you try to become a co-maintainer of that module. I see you say that you've already tried to contact the maintainer; do you have a link to the issue I suppose you raised?

See http://drupal.org/cvs-application/requirements for instructions.

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.

If he does accept you then you, then that folk here should immediately be able to grant you CVS access (once the agreement is documented on the issue queue for that module).

I understand that there is a bit easier working on code 100% on your own, but the benefit to the community is huge to have less modules, but with better/more functionality, so I think this needs to be fully pushed.

@drupalshrek: See my #8 above.

Hi,

I had no idea who Dave Reid was, nor what was said in any private message sent via the contact form.

Dave, since you are here, why don't you think this could be part of your module?

The maintainer already reported what he thinks about merging the proposed module and XML sitemap; we don't usually ask the reason why the maintainer thinks the proposed module could not be merged with the existing project.

Dave Reid is also CVS maintainer, and I think he would consider merging the modules, if this would possible (or in the XML sitemap plans).

OK

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.

DongIT: Please review the steps listed in the Migrating from CVS Applications to (Git) Full Project Applications link above.

To continue with the review process, you will need to set up a Git repository containing your code, comment in this thread with a link to that repository, change the 'Project' field for the issue to 'Drupal.org Project Applications', and update the status to 'needs review'.

As per previous comment, I am setting this issue as Won't fix.
Since new users can now create full projects, applications have a different purpose and they are handled on a different issue queue. See Apply for permission to opt into security advisory coverage for more information.