It would also be nice if there were a way to CC people in review requests. Anyway, since that is not possible, I would like to have you review it too, if you has time, Ronan. You commented something on the issue relating to my patch before I submitted it, and you are generally pretty good with these object oriented design sorts of things.
Just to be clear, I am referring to http://github.com/sympy/sympy/pull/3, which is issue 1985. Aaron Meurer On Tue, Sep 7, 2010 at 3:10 PM, Ondrej Certik <[email protected]> wrote: > On Tue, Sep 7, 2010 at 1:56 PM, Aaron S. Meurer <[email protected]> wrote: >> >> On Sep 7, 2010, at 11:27 AM, Ondrej Certik wrote: >> >>> On Tue, Sep 7, 2010 at 10:26 AM, Ondrej Certik <[email protected]> wrote: >>>> On Sun, Sep 5, 2010 at 11:55 PM, Aaron S. Meurer <[email protected]> >>>> wrote: >>>>> >>>>> On Sep 5, 2010, at 8:26 PM, Aaron S. Meurer wrote: >>>>> >>>>>> >>>>>> On Sep 5, 2010, at 8:21 PM, Aaron S. Meurer wrote: >>>>>> >>>>>>> >>>>>>> On Sep 1, 2010, at 2:48 PM, Ondrej Certik wrote: >>>>>>> >>>>>>>> On Wed, Sep 1, 2010 at 1:18 PM, Aaron S. Meurer <[email protected]> >>>>>>>> wrote: >>>>>>>>> On Aug 31, 2010, at 10:54 PM, Ondrej Certik wrote: >>>>>>>>> >>>>>>>>>> On Tue, Aug 31, 2010 at 9:26 PM, Aaron S. Meurer >>>>>>>>>> <[email protected]> wrote: >>>>>>>>>>> I am not saying that we should move our issue tracker to GitHub (I >>>>>>>>>>> would be opposed to doing that unless there were some way to import >>>>>>>>>>> existing issues from Google Code, and even then only if it could be >>>>>>>>>>> shown that it has all the already existing features plus more to >>>>>>>>>>> make it worth it). >>>>>>>>>> >>>>>>>>>> I agree that we should keep the issues where they are. I meant that >>>>>>>>>> maybe we can use github for code reviews instead of sympy-patches. >>>>>>>>> >>>>>>>>> Oh, OK. Yes, let's try it. >>>>>>>> >>>>>>>> It's in, so now if you want to read the old revieew or comment on it >>>>>>>> (or reopen it), go to: >>>>>>>> >>>>>>>> http://github.com/sympy/sympy/pulls >>>>>>>> >>>>>>>> and click "Closed", or just go here directly: >>>>>>>> >>>>>>>> http://github.com/sympy/sympy/pull/1 >>>>>>>> >>>>>>>> Btw --- github automatically noticed, that I have pushed the patches >>>>>>>> in using "git push" and it closed the pull request. I am getting >>>>>>>> excited for this. :) >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> What I was saying is that there are 78 NeedsReview issues, and no >>>>>>>>>>> way to tell which ones are ones that I have a branch up for. I >>>>>>>>>>> think if we just add the person's user name as a label to an issue >>>>>>>>>>> whenever they add a patch/branch, it will make things easier (I >>>>>>>>>>> would use Owner, but that is already used for other things, and can >>>>>>>>>>> only have one person at a time). >>>>>>>>>>> >>>>>>>>>>> Unless there are any opposed to this idea, I will go ahead and do >>>>>>>>>>> it. >>>>>>>>>> >>>>>>>>>> Go ahead, that'd be awesome! >>>>>>>>> >>>>>>>>> OK. So from now on, whenever an issue has the NeedsReview tag or the >>>>>>>>> PassedReview tag, add the Google Code username of the person who has >>>>>>>>> the patch/branch for review to the issue. It will warn you that you >>>>>>>>> are using an uncommon label, but that is just because I don't want to >>>>>>>>> add everyone's username as an official label. >>>>>>> >>>>>>> So I did this. Now all NeedsReview and PassedReview issues (hopefully) >>>>>>> have the Google Code user name of each person who has a patch/branch >>>>>>> for review as a comment on the issue. Please do the same for any >>>>>>> future issues for which you add this lable, and also please remove the >>>>>>> username if you change NeedsReview to NeedsBetterPatch. >>>>>>> >>>>>>> One note, Google Code will strip the @ from a username that is an >>>>>>> email. So let's just agree to make the label for those users the >>>>>>> username of the email (so instead of [email protected], just >>>>>>> bob.smith). The same I guess will work for users with obfuscated >>>>>>> emails. >>>>>>> >>>>>>> You can easily find the issues for each person by searching for >>>>>>> "label:personname" in the issue tracker. >>>>>>> >>>>>>> Here are some statistics from this. The numbers for the people add up >>>>>>> to more than the totals because some issues have more than one person >>>>>>> who has submitted a patch: >>>>>>> >>>>>>> Number of NeedsReview issues: 77 >>>>>>> smichr: 42 >>>>>>> mattpap: 22 >>>>>>> asmeurer: 7 >>>>>>> Ronan.Lamy: 3 >>>>>>> jensen.oyvind: 3 >>>>>>> nicolas.pourcelot: 2 >>>>>>> Vinzent.Steinberg: 1 >>>>>>> ondrej.certik: 1 >>>>>>> felix.kaiser: 1 >>>>>>> renato.coutinho: 1 >>>>>>> torstenmarcoknodt: 1 >>>>>>> chr.schubert: 1 >>>>>>> >>>>>>> Number of PassedReview issues: 8 >>>>>>> Issues by person: >>>>>>> smichr: 6 >>>>>>> asmeurer: 2 >>>>>>> mattpap: 1 >>>>>>> >>>>>>> I'm not sure what is more embarrassing: 77 NeedsReview issues, many of >>>>>>> which have had that label for months now, or 8 PassedReview issues. >>>>>>> Some of these are part of larger branches, like polys11, but some are >>>>>>> stand alone. Let's push these in! >>>>>>> >>>>>>> And getting back to the original thread, only one of my NeedsReview >>>>>>> issues is for a patch that is separate from my integration3 branch, so >>>>>>> I will add that as a pull request. >>>>>> >>>>>> OK, so when I click on "Pull Request" at GitHub, it wants to notify the >>>>>> user certik (Ondrej) inserted of sympy, assumedly because my repository >>>>>> is set as forked from certik instead of sympy. Any idea how to work >>>>>> around this? >>>>>> >>>>>> By the way, since certik isn't forked from anything (see >>>>>> http://github.com/asmeurer/sympy/network/members) what happens when you >>>>>> click on "Pull Request", Ondrej? >>>>>> >>>>>> Aaron Meurer >>>>> >>>>> Never mind, I figured it out. You just have to change the base commit of >>>>> the comparison to sympy/sympy:master (click on the black commit >>>>> indicator, or "Change Commits"). >>>>> >>>>> So now I've got a pull request in there, if anyone cares. >>>>> >>>>> It doesn't seem to be possible to add a pull request for a single commit >>>>> or small commit range within another branch because, even though you can >>>>> do something like 5a25de2~3..5a25de2 (which is awesome, btw), it is will >>>>> only send the request to the author of 5a25de2~3, which in this case is >>>>> me. I guess I need to go into the GitHub features request and request >>>>> the ability to send the pull request to anybody. >>>> >>>> I noticed your pull request: >>>> >>>> http://github.com/sympy/sympy/pull/3 >>>> >>>> but I don't see it here: >>>> >>>> http://github.com/sympy/sympy/pulls >>>> >>>> is that a github bug, or did you close it? >>> >>> Ah, if you click on the "closed" issues, I can see it. Maybe you >>> should reopen it? Or what is your aim with that. >>> >>> Ondrej >> >> I see what happened. When I added a comment, I clicked on "Comment & Close" >> instead of just "Comment." I didn't know what this meant (I thought "close" >> meant close the window :). > > Yes, my advisor also did this mistake. I think that github guys should > rename it, so I made it: > > http://support.github.com/discussions/site/1893-comment-close-in-pull-requests-is-confusing > > Ondrej > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > 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 [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
