[D7] Datex

Automatic Review

Found issues with Drupal Standards using ventral.org here http://ventral.org/pareview/httpgitdrupalorgsandboxdrupalion1841798git

Manual Review

• Empty .install file in datex_jquery sub-module
• Empty .install file in datex_node_edit sub-module
• Empty .install file in datex_widget sub-module
• Spelling mistake Line 18 in .install file in datex_api sub-module, see $t('PHP Intl Avalibality'),
• In datex_node_edit.js don't you need to have the jQuery function wrapper around the Drupal behaviors? e.g (function ($) { / .. drupal behaviors here .. / })(jQuery);
• In your README.txt you mention "Open common.inc file, replace lines 2008 to 2023" It is not advisable to recommend people changing core Drupal files.

Hi,

I am not sure how you would get around the modifying Drupal core files, maybe that's a question for when you have a review bonus.

As for the jQuery wrapper, I meant the wrapper around all your Drupal.behaviors e.g

(function ($) {

Drupal.behaviors.datexNodeEdit = {
    attach: function(context) {
        jQuery("#edit-date").datetimeEntry({
            datetimeFormat: "Y-O-D H:Ma",
            spinnerImage: "spinnerOrange.png",
            useMouseWheel: true,
            minDatetime: new Date(1900, 1 - 1, 26, 11, 30, 0)
        });
    }
};

})(jQuery);

I put jQuery wrapper around behavior declaration.
Ventral complainings are fixed. (Except a new line at the end of file which is there! but ventral says there is not...)

About the patch: in Drupal's common.inc file (in includes folder), there is a function format_date, all DateTimes are formatted using this function, (eg: displaying authoring date of a node).
The function itself uses PHP DateTime's format function, and PHP-DateTime only supports Gregorian, So, To show a date in Jalali format, the only way is to patch common.inc, And add a hook (using drupal_alter($data, $context)) So other modules get a chance to change the date.

I know it's not something actually called beautiful way of coding, And I spent a lot of time to avoid patching but it's the only way of changing Drupal's default calendar.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

Still working on this module!

You need to set the status to "needs review" if you want to get a review, see https://drupal.org/node/532400

There are many issues found by ventral.org, you should fix most if not all of those: http://ventral.org/pareview/httpgitdrupalorgsandboxdrupalion1841798git.

You should remove the master and HEAD branches from your repo. 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.

You should also remove the .gitignore file unless it's necessary.

Your project page is not very detailed, please have a look at the tips for a great project page, you may also use HTML-tags for better structure.

Please create a README.txt that follows the guidelines for in-project documentation.

In CONTRIBUTORS.txt, include their d.o username and link to their d.o profile (if they have one).

If your module requires a patch to be applied, please include instructions for doing so in an INSTALL.txt file.

In datex_datex_format_alter() I assume the string 'fa' is a language code? Can that be defined as a constant instead and then reused? If someone has installed this module, isn't it safe to assume that they want all their dates altered? Perhaps it is safe- do these dates only make sense when the user has set their language to Persian? The same applies in datex_form_alter().

TODO should probably be TODO.txt.

----
Top Shelf Modules - Enterprise modules from the community for the community.

Ventral.org errors and warnings are fixed again.
Most of them actually. I don't know what to do with " ERROR | Files must end in a single new line character" error, I'm using VIM on Linux, is not putting a newline at the end of file enough? Vim puts a Unix new line character i assume? There is 5 errors about this.

Master and HEAD are removed. They were just there for no purpose!

.gitignore file also removed.

CONTRIBUTORS.txt -> fixed.

README.txt -> added
INSTALL.txt -> In install section of README.txt

about datex_datex_format_alter(),
I don't think it's proper to always display languages in Jalali, It multilingual websites, In English language, date should be Gregorian and in Persian or some other languages which I'm not aware of (Tajiki I think...) Jalali is the official calendar, And may be used.
So I added a form to choose in which languages date should be displayed in Jalali.

And I really don't know what more to say in projects page!

Thanks for those updates! You could use the text from your README on the project page, that would be a great start. And making the language configurable is a good idea.

----
Top Shelf Modules - Enterprise modules from the community for the community.

It's my pleasure,
Do you know how to fix "newline at the end of file error" which ventral is complaining about?

I just added an alternative option to datex, Which will work even if Drupal is not patched with some limited functionality, So if someone can live with not having some options, They won't have to patch core.

End all files with a single new line, some files have two new lines at the end.

I agree with kscheirer making the language configurable is a good idea.

I did make the language configurable! That's what is was trying to explain in commits #9.

And yes thank you nonzod, reading your comment made me doubt if vim is putting 2 \n characters while showing only one of them, I checked with nano and it was the case. I fixed it.

You still have a number of whitespace / syntax issues reported at http://ventral.org/pareview/httpgitdrupalorgsandboxdrupalion1841798git.

Is druplion or lo2y4 applying as the maintainer of this project? We can only promote 1 person to Git Vetted User status? It looks like lo2y4 has all of the recent commits, but the application belongs to drupalion.

----
Top Shelf Modules - Crafted, Curated, Contributed.

I fixed whitespace issues.
yes I've been doing the commits.

If you want us to approve lo2y4 as the git vetted user, we need to hear from drupalion I think.

In datex_api_admin_form() you have

  82 if (!module_exists('libraries')) {
  83     $status = t('Can not use datex jquery, Libraries module is missing.');
  84     $ok = 0;
  85   }
  86   elseif (TRUE) {
  87     $status = t('Can not use datex jquery, Library files are missing from sites/all/libararies.');
  88     $ok = 0;
  89   }
  90   else {
  91     $status = t('Everything is OK and datex widget will work.');
  92     $ok = 1;
  93   }

why the elseif (TRUE) ? that will always execute and the final else will never be evaluated. Not a blocking issue though. I commend you for setting back to "needs review" but this was RTBC on june 30. The issue above is minor, and it's been more than a month.

Thanks for your contribution, lo2y4!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here 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 lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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

Thanks to the dedicated reviewer(s) as well.

----
Top Shelf Modules - Crafted, Curated, Contributed.

datex_api_admin_form() was like that on purpose, To let users know a libraries support was coming, But it doesn't seem like a good idea on a full project.
I'll remove it completely and write about it in project's page.

Thanks everyone for their time and help, Wish you all the best.