On Tue, Jul 6, 2010 at 4:02 PM, Andy Ray Terrel <andy.ter...@gmail.com> wrote: > On Tue, Jul 6, 2010 at 2:33 PM, Ronan Lamy <ronan.l...@gmail.com> wrote: >> Le mardi 06 juillet 2010 à 13:29 -0500, Andy Ray Terrel a écrit : >>> Hello all, >>> >>> Øyvind's fortran code branch is ready to go in, but I did the review >>> through the SmartBear code collaboration server. I will write up how >>> to use the server in more detail later but for people who would like >>> to view the review below is how. Øyvind's branch can be found at [0]. >>> Sorry we didn't include more people on this review, but we wanted to >>> test out the review system first, as you will see in our comments we >>> hit a few snags. If anyone has any comments let us know, otherwise I >>> will push it in tomorrow. >> >> Well, it's a bit hard to understand what's going on. I'd prefer a >> standard review process split into a few issues opened on Google Code >> and, for each, an explanation of what is submitted for inclusion. > > It isn't standard to split a patch review over several issues, but the > use of the review server does not preclude the use of issues. I don't > think any of the SOC students are maintaining an issue for their work, > but if there is a strong preference maybe we should ask them to do so. > > The big advantage for this type of review is that you can see what the > changes are very quickly, respond to a large number of lines for a > variety of issues, and then see how they eventually get fixed. My > experience with the current review is that it becomes very difficult > to follow after about 2 iterations with the code submitter. > >> >> I have only one comment on the code, for now: SymTuple should just be >> called Tuple. > > This is an issue that arrived before this work as SymTuple is already > part of sympy. Other than being outside the scope of this review, I > agree. >
Okay I see the frustration here, the patch moved this into the core. Really we should have a core/containers.py or a containers module then put Set, Tuple, List and other containers we need to box there. > -- Andy > >> >> Ronan >> >> -- >> You received this message because you are subscribed to the Google Groups >> "sympy" group. >> To post to this group, send email to sy...@googlegroups.com. >> To unsubscribe from this group, send email to >> sympy+unsubscr...@googlegroups.com. >> For more options, visit this group at >> http://groups.google.com/group/sympy?hl=en. >> >> > -- You received this message because you are subscribed to the Google Groups "sympy" group. To post to this group, send email to sy...@googlegroups.com. To unsubscribe from this group, send email to sympy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
