[D6] DrupalFever XML Site Map

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxdrupalfever2053413git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Some more spelling errors

Hi Drupal fever,

I am also looking someone who can review my modules.
https://drupal.org/node/2063215

Adding Example page and images

Hi drupalfever,
nice module and nice video tutorial.
I've run Code Sniffer in my local and I've got the follow message:

FILE: ...w/html/drupal/sites/all/modules/test/df_xmlmap/pages/df_xmlmap.page.inc
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 420 | ERROR | CASE keyword must be followed by a single space
 420 | ERROR | Blank lines are not allowed after CASE statements
--------------------------------------------------------------------------------

Don't forget to add the "PAReview: review bonus" tag as indicated in http://drupal.org/node/1975228 , otherwise you won't show up on my high priority list.

Done, thanks!

Hi Drupal fever,

I have download your modules and install on my drupal 6 version, In first look it seem working as expected.
I need more time to re-verify that everything is as expected once I go through your module. will let you know if have any doubt regarding code and api call written in your modules.

Thanks
ajesh

Project status change

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
The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226

  remotes/origin/6.x-1.0

Review of the 6.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. Have you asked the XML Sitemap module maintainers whether you could work on improving the D6 version of that module? Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition.
2. df_xmlmap.info: "package = DrupalFever": what is DrupalFever? Does not seem to be a useful package name to me?
3. df_xmlmap_perm(): So you define a title, but you throw it away with array_keys() in the end? And there is no title property for permission in Drupal 6.
4. df_xmlmap.module: do not globally require your drupalfever_api.inc file. You should only include it when you actually need it, i.e. in function bodies? O r are that important functions that you need on every page request?
5. df_xmlmap_root(): why can't you use gloabl $base_rul? Please add a comment.
6. df_xmlmap_settings(): this function is not necessary, you can drupal_get_form as page callback in hook_menu().
7. df_xmlmap_settings_form(): doc block is wrong, see https://drupal.org/node/1354#forms
8. df_xmlmap_settings_form(): the values of $frequency_options should run through t() for translation, right?
9. df_xmlmap_settings(): all user facing text must run through t() for translation, also #description and words/sentences in #prefix.
10. notice: Undefined index: HTTPS in includes/drupalfever_api.inc on line 73.
11. df_xmlmap_settings_form(): this is vulnerable to XSS exploits. If I have a menu with the title <script>alert('XSS');</script> then I get a nasty javascript popup on the settings page. You need to sanitize all user provided text before printing. Make sure to read https://drupal.org/node/28984 again. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

And removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Hi Drupalfever,

please find my review comment below regarding your modules.

df_xmlmap.module
includes/df_xmlmap.inc

In function like df_xmlmap_cron(),df_xmlmap_output_xml()  Instead of using php function time() you 
should use gmmktime with default timezone time. e.g.

$timezone = variable_get('date_default_timezone', 0);
$c_time = gmmktime()+$timezone;

the above function save data based on current time zone selected in backend.

pages/df_xmlmap.page.inc
df_xmlmap_settings_form()
use t() function for dropdown value, if some one want to translate the value it will be helpful.

and this is my suggestion other wiase this would be fine, I have suggestion using the above form, you can create tpl files and pass all the variable into the tpl files
instead of writing all html tag in this function.

df_xmlmap_regenerate_xml()
in above function you should use dom function instead of assigning xml in variable.
e.g use this function $domDoc = new DOMDocument('1.0', 'utf-8'); for genrating the xml.

include/drupalfever_api.inc

 // If $type property is set to "absolute", return the drupal absolute path.
 elseif ($type == 'absolute') {

  return getcwd() . '/';
}
getcwd() returns the path of the "main" script referenced in the URL.
dirname(__FILE__) will return the path of the script currently executing.
so make sure you have to use the correct function for current working directory.

Tell me your thought on the above review comment. overall the modules is working as expected.