This site is an archive; learn more about 8 years of OpenHatch.

[OH-Dev] Reviewing pull requests

Asheesh Laroia lists at asheesh.org
Mon Jan 13 02:40:00 UTC 2014


Long story short: Merge it as-is, and then add a follow-up commit, and
leave a comment indicating all the reasons you made the cleanups you
did in the follow-up commit.

I'll send a longer note to the list shortly.

On Sun, Jan 12, 2014 at 6:32 PM, John Morrissey <jwm at horde.net> wrote:
> I'm reviewing a pull request, and the changes have a bunch of places where
> the consistency/cruft/style could be better. Nothing fatal, but a general
> tightening over the entire patch.
>
> What do you usually do in cases like these? If I have the submitter fix the
> patch, I'm going to wind up criticizing (politely) pretty evenly over the
> whole thing. Maybe accept it as-is and make a follow-up commit myself
> tightening it up?
>
> john
> --
> John Morrissey          _o            /\         ----  __o
> jwm at horde.net        _-< \_          /  \       ----  <  \,
> www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__
> _______________________________________________
> Devel mailing list
> Devel at lists.openhatch.org
> http://lists.openhatch.org/mailman/listinfo/devel
>


More information about the Devel mailing list