On Aug 18, 2010, at 3:29 PM, Brian Granger wrote: > On Wed, Aug 18, 2010 at 2:22 PM, Ondrej Certik <ond...@certik.cz> wrote: >> Hi Brian, Matt and Addison, >> >> I think the discussion wandered a bit from the review. >> >> 1) Git history: >> >> Do you want to keep your history? Does the history bring any value to >> sympy? From what I understood, many times it doesn't import, or tests >> don't pass. So it seems to me there isn't really any value in keeping >> the history, is there? > > Yes, the history is quite important and I don't see it as an option to > erase it. The reason is attribution and authorship. Each of the 3 of > us authored parts of the branch that is submitted for review and that > information needs to be preserved for copyright/licensing reasons. It > is also important from the standpoint of GSoC legalities, for which we > need to know which student did what.
You're right. Even if you rebase, all the diff files for the GSoC code samples should come from the last HEAD before 2 pm Pacific last Monday. And I would be opposed to creating a single patch that removes the information on who created what (now if there were only one author, it might be a different story, but in that case, there wouldn't be so many merge commits anyway) So you don't care about commit HEADs that don't even import correctly. That's fine. from sympy import * still works, so it is you who will have to deal with the consequences for the future maintenance of the code. (at least personally I don't care about it, since I don't even know anything about quantum mechanics :) But you can still go back and fix commit messages, without changing any of the actual code history. It shouldn't cause any problems with the merge commits, because only the messages will change. Just use the fixup option in git rebase -i in a recent version of git. I would highly recommend doing this. These are for the benefit of other people, and also for your own future self. You may remember what the "Fixed stuff" commit did now, but I guarantee you that you will not remember it a year from now, or even a few months. But if there are more descriptive commit messages, you won't even have to remember what it did, because it will tell you (though the irony is that you probably will remember in that case, because of the more unique and logical message). I would give the same advice that I gave to Christian, which is to back port some of your descriptions from your blog posts into the commit messages (or alternately, you could put these in that doc file some people have suggested). Aaron Meurer > >> Depending on your answer, either: >> >> * create a set of two or three simple patches, that add the new >> functionality, use "git diff" and other things to see what was >> changed, or just this link: >> >> http://github.com/mattcurry/sympy/compare/sympy:master...mattcurry:hilbert2 >> >> as you can see, the real changes are really really simple (just adding >> things and mostly just adding new files). >> >> 2) doctests/documentation >> >> I would like to see this addressed, see my original email. > > Yes, definitely. I will also have a set of review comments myself coming up. > > Cheers, > > Brian > >> >> Ondrej >> >> On Tue, Aug 17, 2010 at 5:21 PM, Aaron S. Meurer <asmeu...@gmail.com> wrote: >>> If you do decide to do rebasing, you should fix up some of the commit >>> messages. "DId [sic] nothing basically," "Not much," and "Nothing" aren't >>> very descriptive. Otherwise, try to do better in the future. You will be >>> cursing yourself later when you go back to fix a bug and can't tell what a >>> commit did because the only description given is "Fixed stuff." >>> >>> Basically, if you are going to have a rebase-free workflow, you have to do >>> commit messages right the first time. >>> >>> Aaron Meurer >>> >>> On Aug 17, 2010, at 1:06 PM, Vinzent Steinberg wrote: >>> >>>> 2010/8/17 Brian Granger <elliso...@gmail.com>: >>>>> On Tue, Aug 17, 2010 at 9:23 AM, Christian Muise >>>>> <christian.mu...@gmail.com> wrote: >>>>>> One thing that was pressed on me was the desire to have every commit >>>>>> run >>>>>> clean. Regardless of what point in the commit history, all tests should >>>>>> work, or at the very least the system should install (broken imports >>>>>> could >>>>>> alter this). I'm currently compiling an exhaustive execution of 'bin/test >>>>>> sympy/physics' for every one of the 140 commits, and many of them don't >>>>>> work. Many have import issues too, which throws a wrench into the mix. >>>>>> I've >>>>>> attached the output until the script I'm using runs into an infinite loop >>>>>> (it tries to keep unrolling the commit history), called output.txt. >>>>> >>>>> For complex code written by 3 people I don't think it is reasonable to >>>>> expect that every commit runs clean. There were many times that one >>>>> of us would commit partially working code that the others would pick >>>>> up and continue with. So I am sure there are many points in the >>>>> history where tests don't pass, code doesn't work or modules are not >>>>> even importable. >>>>> >>>>>> Now I don't think rebasing like mad is really an option -- I only had >>>>>> to >>>>>> deal with dozens of commits for my branch, not 140 :p -- but there needs >>>>>> to >>>>>> be some sort of amalgamation of the commits before it's brought into the >>>>>> trunk. A question for the rest of the community: What's the best way for >>>>>> Matt and Addison to go about achieving this? One single commit of the >>>>>> physics module? >>>>> >>>>> Trying to rebase these commits into a set of self contained commits >>>>> that all run cleanly will be nearly impossible. Also, with three of >>>>> us working on this code, the history of commits and merges is an >>>>> important part of the development process that I don't want to loose. >>>>> It is also important from the attribution and authorship standpoint. >>>>> If we squash all of these into a single commit, all of this >>>>> information is lost. >>>> >>>> Maybe not into a single commit, but it might make sense to squash some >>>> commits. For example "Started another commutator test.", "Fixed >>>> commutator doctest." and "Fixed Commutator issues" could probably be >>>> squashed without problems. But I agree that this is a daunting task >>>> for such a merge-heavy history. >>>> >>>> I'm impressed that you managed to work as 3 people on the same code >>>> base, I think you were able to use git to its fullest potential. :) >>>> >>>> Vinzent >>>> >>>> -- >>>> 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. >>>> >>> >>> -- >>> 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. >>> >>> >> >> -- >> 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. >> >> > > > > -- > Brian E. Granger, Ph.D. > Assistant Professor of Physics > Cal Poly State University, San Luis Obispo > bgran...@calpoly.edu > elliso...@gmail.com > > -- > 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. > -- 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.
