CVS edit link for DongIT

I would like to publish some modules I've created, such as the XML sitemap analyzer which allows one to break down an XML sitemap in a folder tree structure and the Ubercart price attribute multiplier.

Comments

DongIT’s picture

Status:Needs work» Needs review
Issue tags:-module review
StatusFileSize
new4.59 KB

This module is an add-on to the XML sitemap module. It adds a “sitemap analyze” button to the “settings/xmlsitemap” page. It allows users to analyze their generated XML sitemap in a folder tree structure (see screenshot in post bellow). The structure is alphabetically sorted, counted and exapandle folders are grouped and sorted at the bottom of the tree. This module is especially convenient to examine big sitemaps.

kiamlaluno’s picture

Status:Postponed (maintainer needs more info)» Needs work
Issue tags:+module review

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.

DongIT’s picture

Status:Needs review» Needs work
Issue tags:+module review
StatusFileSize
new44.89 KB
DongIT’s picture

Status:Needs work» Needs review
DongIT’s picture

StatusFileSize
new4.57 KB

Used the coder modules to review the code. All warnings have been fixed.

kiamlaluno’s picture

Status:Needs review» Needs work
  • 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. <?php
      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. <?php
     
    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?
DongIT’s picture

Status:Needs work» Needs review
StatusFileSize
new4.66 KB

Hi Kiamlaluno, thank you for your review.

My name is Wouter van Dongen, Drupal/PHP developer for DongIT (http://www.dongit.nl, the website will be translated to English soon). I’ve created the sitemap analyze module because I was looking for a way to be able to analyze a sitemap quicker. Especially with big sitemaps it can be hard to determine if the website has the correct structure and if all URLs are added to the sitemap correctly.

The sitemap analyze module is an add-on to the XML sitemap module (http://drupal.org/project/xmlsitemap). The module adds a “sitemap analyze” button to the “settings/xmlsitemap” page. It allows users to analyze their generated XML sitemap in a folder tree structure were paths can be expanded and collapsed. The structure is alphabetically sorted, counted (direct children and all children) and exapandle folders are grouped and sorted at the bottom of the tree. This module is especially convenient to examine big sitemaps.

I don't believe that any other module provides this functionality. The XML sitemap module adds a stylesheet for the XML sitemap, however this isn’t really convenient for larger sitemaps or to analyze the structure.

I tried to contact the maintainer of the XML sitemap module, but I haven’t had any response.

I've addressed all of the points in the review.

Dave Reid’s picture

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.

kiamlaluno’s picture

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

drupalshrek’s picture

Status:Needs review» Needs work

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.

Dave Reid’s picture

Status:Needs work» Needs review

@drupalshrek: See my #8 above.

drupalshrek’s picture

Status:Needs work» Needs review

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?

drupalshrek’s picture

Status:Needs review» Needs work
kiamlaluno’s picture

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

drupalshrek’s picture

OK

zzolo’s picture

Component:Miscellaneous» miscellaneous
Status:Needs review» Postponed

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’s picture

Title:DongIT [dongit]» XML sitemap analyzer
jthorson’s picture

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