[Ironruby-core] Review: fixes for Array#rindex and Array#reverse_each

Daniele Alessandri suppakilla at gmail.com
Wed Jun 24 19:23:09 EDT 2009


Thanks Jim!

BTW currently my repository is about one month behind. I must merge it with
the main repository but I can't do that now, I hope to make it by tomorrow
in the late afternoon.


On Wed, Jun 24, 2009 at 23:42, Jim Deville <jdeville at microsoft.com> wrote:

> Looks good. Nice job combining the logic. I think I went over this patch,
> but never sent a review mail, sorry about that!
>
> I'll pull it in with the next pull.
>
> JD
>
> …there is no try
>
>
> > -----Original Message-----
> > From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-
> > bounces at rubyforge.org] On Behalf Of Daniele Alessandri
> > Sent: Wednesday, June 24, 2009 1:30 PM
> > To: ironruby-core at rubyforge.org
> > Subject: Re: [Ironruby-core] Review: fixes for Array#rindex and
> > Array#reverse_each
> >
> > Hi,
> >
>  > It seems like this patch has gone unnoticed as it didn't get any review
> yet so
> > here I am, requesting for one :-)
> >
> > Thanks,
> > Daniele
> >
> > On Sun, May 31, 2009 at 13:22, Daniele Alessandri<suppakilla at gmail.com>
> > wrote:
> > > Hi Jim,
> > >
> > > I'm a bit late but daytime work got in the way and then I was out of
> > > the country for two weeks :-) I got rid of the duplicated logic in
> > > IListOps.ReverseIndex and ArrayOps.ReverseEach by implementing a new
> > > internal method
> > > (IListOps.ReverseEnumerateIndexes):
> > >
> > >
> > http://github.com/nrk/ironruby/commit/76db817d1766b788f995afb02e12c5
> > a7
> > > 2955c77f
> > >
> > > As for the specs, I just slightly modified an existing one in a way
> > > that does not affect the test but helped me to discover the bug
> > > (actually this change uses a different condition from the one I used
> > > one month ago as it exposed another bug which got fixed in the above
> > > mentioned commit):
> > >
> > >
> > http://github.com/nrk/ironruby/commit/e5497cf87fc479b2bf2ca0b812d9793
> > 5
> > > 4a86f44c
> > >
> > > See also the attached diff.
> > >
> > > Thanks,
> > > Daniele
> > >
> > >
> > > On Thu, Apr 23, 2009 at 22:08, Jim Deville <jdeville at microsoft.com>
> wrote:
> > >> Can you add specs for rindex that expose the bug you fixed? Also, is
> there
> > any shared place that you could put the following code:
> > >>         if (self.Count < originalSize) {
> > >>             i = originalSize - i - 1 + self.Count;
> > >>             originalSize = self.Count;
> > >>         }
> > >>
> > >> It would be nice to get rid of the duplicated logic, but I can't think
> of
> > where it should go.
> > >>
> > >> Other than that, looks good.
> > >> JD
> > >>
> > >>> -----Original Message-----
> > >>> From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-
> > >>> bounces at rubyforge.org] On Behalf Of Daniele Alessandri
> > >>> Sent: Thursday, April 23, 2009 12:59 PM
> > >>> To: ironruby-core at rubyforge.org
> > >>> Subject: [Ironruby-core] Review: fixes for Array#rindex and
> > >>> Array#reverse_each
> > >>>
> > >>> Hi,
> > >>> I just pushed two fixes on my repository, the first one addresses a
> > >>> bug in Array#rindex (there was a bug in my last commit) and the
> > >>> second one makes Array#reverse_each compliant with the rubyspecs.
> > >>>
> > >>>
> > http://github.com/nrk/ironruby/commit/d2b18f5d01a49cb62a2ea0c205e1cf
> > >>> 123
> > >>> 3ac94e0
> > >>>
> > >>> >From the commit message:
> > >>>
> > >>> * Fixed a bug in IListOps.ReverseIndex (core/array/rindex specs were
> > >>> passing, this bug was triggered under certain conditions different
> > >>> from the ones defined in the specs)
> > >>>
> > >>> * Fixed ArrayOps.ReverseEach to make it not fail when elemens in the
> > >>> array are removed from inside a block.
> > >>>
> > >>> See also the attached diff.
> > >>>
> > >>> Thanks,
> > >>> Daniele
> > >>>
> > >>> --
> > >>> Daniele Alessandri
> > >>> http://www.clorophilla.net/blog/
> > >>> http://twitter.com/JoL1hAHN
> > >> _______________________________________________
> > >> Ironruby-core mailing list
> > >> Ironruby-core at rubyforge.org
> > >> http://rubyforge.org/mailman/listinfo/ironruby-core
> > >>
> > >
> > >
> > >
> > > --
> > > Daniele Alessandri
> > > http://www.clorophilla.net/blog/
> > > http://twitter.com/JoL1hAHN
> > >
> >
> >
> >
> > --
> > Daniele Alessandri
> > http://www.clorophilla.net/blog/
> > http://twitter.com/JoL1hAHN
> > _______________________________________________
> > Ironruby-core mailing list
> > Ironruby-core at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/ironruby-core
> _______________________________________________
> Ironruby-core mailing list
> Ironruby-core at rubyforge.org
> http://rubyforge.org/mailman/listinfo/ironruby-core
>



-- 
Daniele Alessandri
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20090625/8753fbac/attachment.html>


More information about the Ironruby-core mailing list