On Fri, Sep 3, 2010 at 3:50 PM, Aaron S. Meurer <asmeu...@gmail.com> wrote:
> On Sep 3, 2010, at 4:21 PM, Ondrej Certik wrote:
>
>> Hi,
>>
>> I have pushed this in, so the blame goes to my head. Some comments:
>>
>> On Fri, Sep 3, 2010 at 10:16 AM, Christian Muise
>> <christian.mu...@gmail.com> wrote:
>>>> This is indeed very strange.  According to issue 2046 (and my own
>>>> bisecting as well) it comes from this commit:
>>>>
>>>> commit dcbc2da31324e98c9cb3a4bf17c50f029774ae06
>>>> Author: Sebastian Krämer <basti...@gmail.com>
>>>> Date:   Wed May 5 22:48:18 2010 +0200
>>>>
>>>>    Implement fast atomic substitution.
>>>>
>>>>    * The main code is in Basic._subs_dict
>>>>    * It is used automatically by subs when possible.
>>>>    * Where the default substitution is not good enough this method is
>>>>      overridden.
>>>>    * Because of hashing problems (different hash although equal) one
>>>>      example in the modules documentation has to be changed.
>>>>    * This commit only couples it very loose to subs. By rewriting subs a
>>>>      little and integrating it better there will be some more speed
>>>>      improvement.
>>>
>>>   It looks like the pull into the main repo (sympy/sympy/master) did some
>>> wonky things. Attached is a view of gitk from my local master. Maybe a
>>
>> No, it was there in your pull request, I remember it, and you can
>> still check it easily here:
>>
>> http://github.com/sympy/sympy/pull/2
>>
>> I thought it was somehow needed for your branch, and also you wrote
>> there (feel free to go on that link to check it):
>>
>> """
>> As per discussion 0 this branch, containing the modifications over the
>> summer for Google's SoC program, is ready to be merged in. A number of
>> iterations has already gone into getting it up to shape, but I'm open
>> to more suggestions if there are any.
>>
>> Thanks. Cheers
>> """
>>
>> So I somehow believed the branch is ok to go in. Which is my fault,
>> and I apologize. Next time I'll wait to get approvals from the
>> reviewers too, so that they can say +1 or -1 to the final branch going
>> in.
>
> Maybe we should have some rule that requires more than one reviewer to sign 
> off for large branches such as this one.

In this case I think it would help to get final +1 on the final branch
going in, so that we can eliminate last minute mistakes with rebasing.
What I should have waited.

>
>>
>> Aaron, can you please revert that patch?
>
> I already fixed everything earlier today.  I actually screwed up myself 
> because I didn't realize that SymTuple was moved to the core, so I had to 
> partially revert my reversion.  But everything should be good now, at least 
> with respect to that (unless some other commit snuck in there).

Thanks!
Ondrej

-- 
You received this message because you are subscribed to the Google Groups 
"sympy-patches" group.
To post to this group, send email to sympy-patc...@googlegroups.com.
To unsubscribe from this group, send email to 
sympy-patches+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/sympy-patches?hl=en.

Reply via email to