Rajesh Taneja added a comment - 28/Mar/12 3:34 PM Increasing priority as it is broken functionality and can affect multiple modules.

Rajesh Taneja added a comment - 29/Mar/12 5:33 PM https://github.com/rajeshtaneja/moodle/blob/6efbcd9257b34c1cc51c5856fd7ba43f806f590f/lib/moodlelib.php#L2074 getdate array should match with one produced by $datestring = gmstrftime('%B_%A_%j_%Y_%m_%w_%d_%H_%M_%S', $time); %m returns two digit representation of the month, whereas getdate returns numeric representation of a month. In theory it should not break anything, but still think it should go in master only and then back-ported later to stable branches.

Dan Poltawski added a comment - 30/Mar/12 10:54 AM Hi Raj, I agree about the change because usergetdate should not return different formatted results if called without a timezone. (Which it does by calling getdate which returns month without the leading 0). A really trivial comment about thing about the commit message ( http://docs.moodle.org/dev/Commit_cheat_sheet ) - could you shortern the first line and include mention of the function name. It helps in viewing the logs to have that shortened and including the function name just helps when viewing history to know exactly where it affected

Rajesh Taneja added a comment - 30/Mar/12 11:58 AM Thanks Dan, Commit message is shortened now. Pushing it for integration review.

Dan Poltawski added a comment - 30/Mar/12 12:06 PM Hi Raj - you didn't need to shortern the commit message, just first line I really encourage you to write detailed messages in the next lines!

Aparup Banerjee added a comment - 03/Apr/12 11:37 AM - edited Hi Raj, this looks good, it seems other calls won't be affected or will cast into integer. i was wondering why not strip for %d too ( Two-digit day of the month (with leading zeros) ) as the same index error could be avoided elsewhere not to mention the format is kept same as getdate(). $datestring = gmstrftime('%B_%A_%j_%Y_%-m_%w_%-d_%H_%M_%S', $time); ?

Rajesh Taneja added a comment - 03/Apr/12 2:03 PM Thanks Aparup, You are right this should have been checked for all the values. Following is fixed now: All int values are stripped with leading zero. Values are typecasted properly, before they return %j returns 001 through 366, whereas getdate() returns 0 through 365. This has been taken care off. I have added 21 and 22 branches as well, and it should go on stable as well.

Dan Poltawski added a comment - 03/Apr/12 2:23 PM All looks good here, but this is slightly more scary change now. Perhaps a deep look of uses is required.

Aparup Banerjee added a comment - 03/Apr/12 2:30 PM reopening for more work (going on at the moment too)

Rajesh Taneja added a comment - 16/Apr/12 10:26 AM I think this is good to go. Dan can you please review this again for me. Thanks

Dan Poltawski added a comment - 17/Apr/12 1:53 PM Looks good to me

Rajesh Taneja added a comment - 17/Apr/12 2:13 PM Thanks Dan, Pushing it for integration.

Dan Poltawski added a comment - 19/Apr/12 9:57 PM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao

Rajesh Taneja added a comment - 20/Apr/12 1:40 PM Branches re-based.

Eloy Lafuente (stronk7) added a comment - 24/Apr/12 7:46 PM Hi Rajesh, I was moving your changes in (not existing anymore) lib/simpletest/testmoodlelib.php to the new lib/tests/moodlelib_test.php and executing it I'm getting one failure: phpunit lib/tests/moodlelib_test.php   Time: 3 seconds, Memory: 71.00Mb   There was 1 failure:   1) moodlelib_testcase::test_usergetdate Failed asserting that 7 matches expected 0.   integration/lib/tests/moodlelib_test.php:1016 integration/lib/phpunit/lib.php:1162 So it seems that the code is introducing some problem with the seconds... so I'm reopening this. Ciao

Eloy Lafuente (stronk7) added a comment - 24/Apr/12 9:58 PM - edited Hi Rajesh, it seems that we cannot rely in the "-" modifier at all. I asked @ HQ for help running this: php -r 'error_reporting(0); date_default_timezone_set("UTC"); foreach (str_sp lit("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrfti me("%-$m", 7) . "#" . strftime("%$m", 7) . "#" . strftime("%-$m", 7) . "#" . PHP_EOL; }' And got all sort of results, in summary: Linux seemed to accept it ok. MacOS X was working ok for Eric but not for me (I was getting "-S" on output) Windows directly "eat" any modifier with "-" (and requires setting error reporting to 0. Solaris returns "-S" too. So I think we must do that without those "-". Here it's a copy of the discussion @ HQ. Thanks everybody: [15:48:36] <Cindereloy> Offtopic. Can anybody execute this under linux and windows:   php -r 'date_default_timezone_set("UTC"); foreach (str_split("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrftime("%-$m", 7) . "#" . strftime("%$m", 7) . "#" . strftime("%-$m", 7) . "#" . PHP_EOL; }'   and paste results? [15:48:49] <Cindereloy> (it's not a virus) [15:49:28] <Eric Merrill> B: January#January#January#January# A: Thursday#Thursday#Thursday#Thursday# j: 001#1#001#1# Y: 1970#1970#1970#1970# m: 01#1#01#1# w: 4#4#4#4# d: 01#1#01#1# H: 00#0#00#0# M: 00#0#00#0# S: 07#7#07#7# [15:49:37] <Cindereloy> that is which OS? [15:49:44] <Eric Merrill> RHL 5 I think [15:49:52] <timhunt> repeated many thousands of times. [15:50:15] <Eric Merrill> Identical on mac os x too [15:50:18] <timhunt> How can I kill the warnings and notices? [15:50:22] <Cindereloy> identical on mac? [15:50:24] <david> B: January#January#January#January# A: Thursday#Thursday#Thursday#Thursday# j: 001#1#001#1# Y: 1970#1970#1970#1970# m: 01#1#01#1# w: 4#4#4#4# d: 01#1#01#1# H: 00#0#00#0# M: 00#0#00#0# S: 07#7#07#7#   [15:50:34] <Cindereloy> grrr, here i get:   B: January#-B#January#-B# A: Thursday#-A#Thursday#-A# j: 001#-j#001#-j# Y: 1970#-Y#1970#-Y# m: 01#-m#01#-m# w: 4#-w#4#-w# d: 01#-d#01#-d# H: 00#-H#00#-H# M: 00#-M#00#-M# S: 07#-S#07#-S#   [15:50:37] <Eric Merrill> Red Hat and Mac seem to give identical results [15:51:08] <timhunt> $ php -r 'error_reporting(0); date_default_timezone_set("UTC"); foreach (str_sp lit("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrfti me("%-$m", 7) . "#" . strftime("%$m", 7) . "#" . strftime("%-$m", 7) . "#" . PHP_EOL; }' B: January##January## A: Thursday##Thursday## j: 001##001## Y: 1970##1970## m: 01##01## w: 4##4## d: 01##01## H: 00##00## M: 00##00## S: 07##07## [15:51:25] <Cindereloy> buf, that's worse. It "eats" them. [15:51:26] <timhunt> I added error_reporting 0 [15:51:48] <Cindereloy> oki, thanks. I think the conclusion is that we cannot rely on the "-" modifier. [15:51:52] <Andrew Nicols> Do you want it on Solaris too? [15:51:58] <Cindereloy> yes thanks [15:52:03] <Andrew Nicols> 504 nicols@current10x:~> /opt/csw/php5/bin/php -r 'date_default_timezone_set("UTC"); foreach (str_split("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrftime("%-$m", 7) . "#" . strftime("%$m", 7) . "#" . strftime("%-$m", 7) . "#" . PHP_EOL ; }' B: January#%-B#January#%-B# A: Thursday#%-A#Thursday#%-A# j: 001#%-j#001#%-j# Y: 1970#%-Y#1970#%-Y# m: 01#%-m#01#%-m# w: 4#%-w#4#%-w# d: 01#%-d#01#%-d# H: 00#%-H#00#%-H# M: 00#%-M#00#%-M# S: 07#%-S#07#%-S#   [15:52:09] <Cindereloy> aha [15:52:28] <Cindereloy> oki, many thanks.

Rajesh Taneja added a comment - 26/Apr/12 10:54 AM Thanks for pointing this Eloy I have updated all branches.

Eloy Lafuente (stronk7) added a comment - 27/Apr/12 11:15 PM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao

Rajesh Taneja added a comment - 30/Apr/12 11:10 AM Branches re-based

Eloy Lafuente (stronk7) added a comment - 03/May/12 6:55 AM Integrated, thanks! (21, 22 & master). Note: I've added one extra commit on each branch to change current assertEqual() tests to assert [Identical|Same] () (simpletest|phpunit), so both value and type will be tested.

Jason Fowler added a comment - 03/May/12 2:30 PM all good raj, thanks for the clear test instructions

Eloy Lafuente (stronk7) added a comment - 05/May/12 5:41 PM UPDATE tracker_issues SET status = 'Closed', comment = 'Thanks!' WHEN participants = 'Did a gorgeous work' This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).