On Tue, Aug 16, 2011 at 9:04 PM, Ted Yu <[email protected]> wrote: > Seeking a little clarification for request #2: > Let us assume the good patch is submitted on day X > Committer A put +1 on the patch on day Y (>= X) > Committer B put +1 on the patch on day Z (> Y) > > Is it Okay to commit the patch on or after Z ?
I think it's a judgment call - I have no problem with trivial patches being committed "immediately" - ie on first +1, or even before if it's truly trivial. On the other hand, something like HFile v2, Li's block cache, largely rewritten HLog, etc, should have a review period of several days if not more. I trust the committers to make the call here - no sense in having hard rules. In my mind, some of the reasons why a patch should need more review are: - very large patch (people might be putting off the review for a few days to find a 3-4 hour chunk of time to look at it) - non-trivial changes to really core/complicated bits (eg HLog, MemStore, etc) - big API changes (we'll have to maintain them going forward) If a committer is on the edge of whether something needs more review, maybe send a quick note to dev@ along the lines of "I plan to commit HBASE-1234 tomorrow unless there are any objections. Is anyone still planning on taking a look who hasn't yet?" -Todd > On Tue, Aug 9, 2011 at 1:52 PM, Todd Lipcon <[email protected]> wrote: > >> Hey folks. Some time over the last three years working on Hadoop and >> HBase I've turned from a fun loving youth into a grumpy old man. So >> here are two grumpy requests I've been thinking about of late, on >> which I'd like to solicit opinions. >> >> Grumpy request #1 >> ------------------------------ >> >> I commented the following on HBASE-2077 earlier, but figured it was >> worth putting on the mailing list as well. >> >> "In the future could we open separate JIRAs rather than doing a "part >> 2" when the commits are more than a day apart? It's very difficult to >> figure out what went on in the history of this JIRA, since it was >> committed for 0.20 in Dec '09, briefly amended in Feb '10, amendation >> partially reverted the next day, and then another change in Jun '11 >> for 0.90.4 to solve an entirely different bug than the description >> indicates. This makes it very difficult to support past branches or >> maintain distributions, since it appears this was fixed long ago but >> in fact 0.90.3 lacks a major part of the JIRA." >> >> This happens fairly frequently in HBase (I'm guilty of it as well), >> and while I appreciate the value of a lightweight process, it does >> make it very difficult to track bug fixes when the multiple commits >> can cross different point releases. For example, we often have >> customers who have heard of some JIRA number fixing a problem, and say >> "does 0.90.3 include HBASE-nnnn"? A quick look at the history of >> 0.90.3 will tell you "yes", when in fact the real underlying issue >> isn't fixed until 0.90.4, trunk, etc. >> >> I'd like to propose the following guidelines for "followup commits >> under the same JIRA": >> 1) A followup commit is fine so long as it follows within 1 day of the >> original commit. >> 1a) The followup commit should include in the commit message a >> description of what differs, eg a commit message format like: >> "Amend HBASE-nnnn. Followup to previous commit which forgot to include >> Foo.java" would be great. Or if it fixes some small bug in the >> previous commit, "Amend HBASE-nnnn. Fix bug JD pointed out in >> http://permalink-to-the-jira-comment";. >> 2) A followup commit may never be done "across versions" - ie if a >> JIRA has already been committed to any code base that's been released, >> it can't be amended after that, even if the fix is trivial. >> 3) Backport commits are of course OK so long as the patch is >> essentially the same (eg if something's committed to 0.92.0, it can be >> backported to 0.90.n if it's basically the same code) >> >> Does this seem reasonable? >> >> Grumpy request #2 >> ----------------------------- >> Recently we've had a lot of great significant contributions. A lot of >> the time they've been committed very quickly -- ie from patch to >> commit in a few hours. This is great for encouraging contributors, but >> I'm worried we may lose some stability or may introduce features that >> are questionable. For any patches that are complicated or introduce >> new APIs, can we try to leave them open for at least a day or two >> before commit? >> >> >> Thanks >> -Todd >> -- >> Todd Lipcon >> Software Engineer, Cloudera >> > -- Todd Lipcon Software Engineer, Cloudera
