updated removing checks, now is ready to pull
On Tue, Aug 2, 2011 at 8:27 PM, Enrico Sada <enrico.s...@gmail.com> wrote: > yes the test was red before. > > i will remove the checks for 1.9 (are useless) and update the pull request. > > On Tue, Aug 2, 2011 at 7:20 PM, Jimmy Schementi <jscheme...@gmail.com> wrote: >> On Tue, Aug 2, 2011 at 12:44 PM, Enrico Sada <enrico.s...@gmail.com> wrote: >>> >>> i asked a pull request ( https://github.com/IronLanguages/main/pull/28 >>> ) for fix Time#strftime behaviour on invalid directives on format >>> string, ex: >>> >>> ruby 1.8: >>> Time.now.strftime '%$' => '$' >>> ruby 1.9: >>> Time.now.strftime '%$' => '%$' >>> >>> this make green a mspec test on core/time/strftime_spec.rb >> >> Looks good. Did the test fail before ths change? >> >>> >>> I have some question: >>> 1) i added a check for ruby compatibility >= ruby 1.9, is needed? >> >> Not really as we have explicitly decided that IronRuby 1.x targets MRI 1.9, >> and doesn't support 1.8.x anymore. However, we didn't remove any of those >> checks for 1.8. It doesn't hurt, but isn't required. >> >>> >>> 2) i removed 'fails:' from >>> ironruby-tags-19/core/time/strftime_tags.txt is correct? >> >> Yup, you fixed that test so no need for the guard anymore. >> >>> >>> 3) usually code review is in mailing list or github pull request? (so >>> i dont need to write two times the same questions) >> >> It doesn't matter where you actually write the text, but the pull request is >> needed as well as notifying the mailing list. I say put everything in the >> pull request, and then send the link to the pull request to the mailing >> list. >> >>> >>> _______________________________________________ >>> Ironruby-core mailing list >>> Ironruby-core@rubyforge.org >>> http://rubyforge.org/mailman/listinfo/ironruby-core >> >> >> _______________________________________________ >> Ironruby-core mailing list >> Ironruby-core@rubyforge.org >> http://rubyforge.org/mailman/listinfo/ironruby-core >> >> > _______________________________________________ Ironruby-core mailing list Ironruby-core@rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core
