Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened. When I try to merge the changes to upstream, they get rejected. I can merge it back to my own master, and can even push it up to my github repo’s master, but not matplotlib’s master.
After some investigation, I am left wondering if the cause might be that my branches were branched off of my (un-updated) master (which tracked my github’s master, not matplotlib’s master). I have tried rebasing my branches, but that doesn’t seem to solve the problem.
I am thoroughly confused. Anybody has ideas? Should I dump my repos and start fresh?
Thanks,
Ben Root
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
When I try
to merge the changes to upstream, they get rejected. I can merge it back to
my own master, and can even push it up to my github repo's master, but not
matplotlib's master.
After some investigation, I am left wondering if the cause might be that my
branches were branched off of my (un-updated) master (which tracked my
github's master, not matplotlib's master). I have tried rebasing my
branches, but that doesn't seem to solve the problem.
I am thoroughly confused. Anybody has ideas? Should I dump my repos and
start fresh?
No, don't do that. Which repository are you trying to push to? You
probably need to sync up with the remote. See the discussion on "git
push" at the end of http://gitref.org/remotes/
···
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root <ben.root@...553...> wrote:
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
Ryan
···
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root <ben.root@...553...> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
--
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma
I agree. Hence the "in general" qualification.
···
On Fri, Feb 25, 2011 at 2:43 PM, Ryan May <rmay31@...149...> wrote:
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root <ben.root@...553...> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
Sorry. Glossed over that and took you mentioning it in this context as
objecting to self-merging this particular set of simple changes.
Ryan
···
On Fri, Feb 25, 2011 at 1:48 PM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 2:43 PM, Ryan May <rmay31@...149...> wrote:
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root <ben.root@...553...> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
I agree. Hence the "in general" qualification.
--
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma
I just want to throw out there that in the migration to github, we
never officially said we were going to switch the development process.
In fact, we said the opposite. After the migration, Jarrod suggested
the pull request workflow as espoused in gitwash, and I am happy to
experiment with it, but only to the extent that "it works", ie we are
getting fast enough code reviews and pull requests closed that
development is slowed significantly. In our experience on sf, we
weren't doing a good job keeping up with submitted requests by
non-developers on the trackers, much less reviewing the core devs'
contributions. Let he who thinks they can keep up with MD and JJ step
forward...
JDH
···
On Fri, Feb 25, 2011 at 1:43 PM, Ryan May <rmay31@...149...> wrote:
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root <ben.root@...553...> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
FWIW, my take on this question from the same conversation on
ipython-dev a few days ago:
http://mail.scipy.org/pipermail/ipython-dev/2011-February/007258.html
Others seemed OK with that approach.
HTH,
f
···
On Fri, Feb 25, 2011 at 11:48 AM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 2:43 PM, Ryan May <rmay31@...149...> wrote:
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
I agree. Hence the "in general" qualification.
Yo,
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
I agree. Hence the "in general" qualification.
FWIW, my take on this question from the same conversation on
ipython-dev a few days ago:
http://mail.scipy.org/pipermail/ipython-dev/2011-February/007258.html
Others seemed OK with that approach.
In nipy, we really haven't got into the swing of code review, but I
see sympy going for it with enthusiasm, and they're better than us 
Our policy thus far has been:
doc changes : go for it
small bug fix with test : use judgment, probably go for it
anything else : post and ask for review. In general (TM). No review,
after some period, perhaps with reminder, go for it.
It may not be very obvious, but the wait-for-review step has far less
inconvenient consequences using git than svn because it's so easy to
merge, rebase and so on.
(lurking now resumed),
Matthew
···
On Fri, Feb 25, 2011 at 12:02 PM, Fernando Perez <fperez.net@...149...> wrote:
On Fri, Feb 25, 2011 at 11:48 AM, Darren Dale <dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 2:43 PM, Ryan May <rmay31@...149...> wrote:
One comment from the peanut gallery: what we've found in ipython is
that the git pull request system does make the review process vastly
more efficient. The tool is good enough that we've been reviewing
things pretty quickly, and it does overall help the project's code
quality quite a bit. It makes a big difference if doing a review has
high overhead or if it's just a matter of clicking on a link, having
all the information nicely presented there for you (conversation,
files changed, highlighted diff), and being able to comment in 2
minutes.
For simple things often my review amounts to 'good job, merge away but
fix this little thing I commented on inline'. It takes me 2 minutes
to do it, I manage to get in some feedback that improves the code
before merge, and I don't even ever download the actual branch, since
I do all the reviewing (for simple ones) just from the diffs on the
github pages.
Cheers,
f
···
On Fri, Feb 25, 2011 at 12:01 PM, John Hunter <jdh2358@...149...> wrote:
I just want to throw out there that in the migration to github, we
never officially said we were going to switch the development process.
In fact, we said the opposite. After the migration, Jarrod suggested
the pull request workflow as espoused in gitwash, and I am happy to
experiment with it, but only to the extent that "it works", ie we are
getting fast enough code reviews and pull requests closed that
development is slowed significantly. In our experience on sf, we
weren't doing a good job keeping up with submitted requests by
non-developers on the trackers, much less reviewing the core devs'
contributions. Let he who thinks they can keep up with MD and JJ step
forward...
I did a pull request for this minor change because I am still learning the ins and outs of git and decided to make this commit in the manner I was already familiar.
My feeling is that these pull requests are a great way to group various commits into logical chunks. It also makes for a good “paper” trail for all changes. Github provides so many useful tools surrounding them that making direct merges without them seems almost disruptive. I am worried that having two different workflows may cause issues down the line.
My take is that pull requests should always be done, but we should leave it to the discretion of the core developers whether or not they can self-close. This is not too different from how things were done on SF. I personally would send out an email for more significant changes, wait a week, do a ping, and then self-commit if there were no comments. For typos and such, I did not even bother asking for reviews (although this was rare).
Just my two cents,
Ben Root
P.S. - Darren, my hat is off to you for all your work lately. I can’t thank you enough.
···
On Fri, Feb 25, 2011 at 2:01 PM, John Hunter <jdh2358@…149…> wrote:
On Fri, Feb 25, 2011 at 1:43 PM, Ryan May <rmay31@…149…> wrote:
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale <dsdale24@…149…> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root <ben.root@…553…> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don’t think we should close our own pull requests. It
short-circuits the review process.
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there’s a point of diminishing returns.
I just want to throw out there that in the migration to github, we
never officially said we were going to switch the development process.
In fact, we said the opposite. After the migration, Jarrod suggested
the pull request workflow as espoused in gitwash, and I am happy to
experiment with it, but only to the extent that “it works”, ie we are
getting fast enough code reviews and pull requests closed that
development is slowed significantly. In our experience on sf, we
weren’t doing a good job keeping up with submitted requests by
non-developers on the trackers, much less reviewing the core devs’
contributions. Let he who thinks they can keep up with MD and JJ step
forward…
JDH
Elaborating a bit: mpl historically has had a very loose management and a free approach to changes (Thanks, John!). I think it has served us well. The move to git permits a continuation of this style (though for a smaller set of committers), while facilitating more structured procedures; it doesn't force us to change, it makes it easier to use a range of styles, as appropriate, and perhaps gently nudges us towards more review prior to committing.
As always, review is not a one-time opportunity. Committed changes always can be reviewed, and new changes made as needed.
This is evolution, not revolution.
Eric
···
On 02/25/2011 10:01 AM, John Hunter wrote:
On Fri, Feb 25, 2011 at 1:43 PM, Ryan May<rmay31@...149...> wrote:
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale<dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root<ben.root@...553...> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
I just want to throw out there that in the migration to github, we
never officially said we were going to switch the development process.
In fact, we said the opposite. After the migration, Jarrod suggested
the pull request workflow as espoused in gitwash, and I am happy to
experiment with it, but only to the extent that "it works", ie we are
getting fast enough code reviews and pull requests closed that
development is slowed significantly. In our experience on sf, we
weren't doing a good job keeping up with submitted requests by
non-developers on the trackers, much less reviewing the core devs'
contributions. Let he who thinks they can keep up with MD and JJ step
forward...
JDH
I'll back off my assertion somewhat, and instead observe that, so far,
pull requests and associated code review have been working out
superbly. We are catching problems before they find their way into the
official repos. People are making constructive criticism, making
encouraging comments. Perhaps my original comment was too rigid, and
instead I should say that the pull request/code review workflow is
shaping up to be a really code thing, and I would like to advocate for
its continued use. Where appropriate.
···
On Fri, Feb 25, 2011 at 3:16 PM, Eric Firing <efiring@...229...> wrote:
On 02/25/2011 10:01 AM, John Hunter wrote:
On Fri, Feb 25, 2011 at 1:43 PM, Ryan May<rmay31@...149...> wrote:
On Fri, Feb 25, 2011 at 1:11 PM, Darren Dale<dsdale24@...149...> wrote:
On Fri, Feb 25, 2011 at 1:45 PM, Benjamin Root<ben.root@...553...> wrote:
Ok, I am still learning quite a bit about git, please bear with me.
I am having difficulty completing a pull request that I opened.
In general, I don't think we should close our own pull requests. It
short-circuits the review process.
Agreed in principle. However, do we as devs want to get/give reviews
on every change that fixes typos in the docs or fixes stupid bugs in
examples? I think there's a point of diminishing returns.
I just want to throw out there that in the migration to github, we
never officially said we were going to switch the development process.
In fact, we said the opposite. After the migration, Jarrod suggested
the pull request workflow as espoused in gitwash, and I am happy to
experiment with it, but only to the extent that "it works", ie we are
getting fast enough code reviews and pull requests closed that
development is slowed significantly. In our experience on sf, we
weren't doing a good job keeping up with submitted requests by
non-developers on the trackers, much less reviewing the core devs'
contributions. Let he who thinks they can keep up with MD and JJ step
forward...
JDH
Elaborating a bit: mpl historically has had a very loose management and
a free approach to changes (Thanks, John!). I think it has served us
well. The move to git permits a continuation of this style (though for
a smaller set of committers), while facilitating more structured
procedures; it doesn't force us to change, it makes it easier to use a
range of styles, as appropriate, and perhaps gently nudges us towards
more review prior to committing.
As always, review is not a one-time opportunity. Committed changes
always can be reviewed, and new changes made as needed.
This is evolution, not revolution.