Re: B-Tree support function number 3 (strxfrm() optimization) - Mailing list pgsql-hackers
From | Stephen Frost |
---|---|
Subject | Re: B-Tree support function number 3 (strxfrm() optimization) |
Date | |
Msg-id | 20140407175834.GV4582@tamriel.snowman.net Whole thread Raw |
In response to | Re: B-Tree support function number 3 (strxfrm() optimization) (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: B-Tree support function number 3 (strxfrm() optimization)
|
List | pgsql-hackers |
* Robert Haas (robertmhaas@gmail.com) wrote: > If it's only going to take you an hour to address this patch (or 8 to > address those other ones) then you spend a heck of a lot less time on > review for a patch of a given complexity level than I do. Eh, I don't really time it and I'm probably completely off in reality, it's more of a relative "feeling" thing wrt how long it'd take. I was also thinking about it from a 'initial review and provide feedback' standpoint, actually getting to a point of committing something certainly takes much longer, but I can also kick off tests and do individual testing in those smaller time blocks. Reading the code and understanding it, writing up the feedback email, etc, is what requires the larger single block. Perhaps I can work on improving that for myself and maybe find a way to do it in smaller chunks, but that hasn't happened in the however-many-years, and there's the whole 'old dog, new tricks' issue. > I agree > that it's desirable to slip things in, from time to time, when they're > uncontroversial and obviously meritorious, but I'm not completely > convinced that this is such a case. As an utterly trivial point, I > find the naming to be less than ideal: "poorman" is not a term I want > to enshrine in our code. That's not very descriptive of what the > patch is actually doing even if you know what the idiom means, and > people whose first language - many of whom do significant work on our > code - may not. Fair enough. > Now the point is not that that's a serious flaw in and of itself. The > point is that these kinds of issues deserve to be discussed and agreed > on, and the process should be structured in a way that permits that. The issue on it being called "poorman"? That doesn't exactly strike me as needing a particularly long discussion, nor that it would be difficult to change later. I agree that there may be other issues, and it'd be great to get buy-in from everyone before anything goes in, but there's really zero hope of that being a reality. > And that discussion will require the time not only of the people who > find this patch more interesting than any other, but also of the > people who just said that they're busy with other things right now, > unless those people want to forfeit their right to an opinion. I certainly hope that no committer feels that they forfeit their right to an opinion about a piece of code because they didn't object to it before it was committed. My experience is that committed code gets reviewed and concerns are raised, at which point it's usually on the original committer to go back and fix it; which I'm certainly glad for. > Experience has shown that it's a whole lot easier for anyone here to > get a patch changed before it's committed than after it's committed, > so I don't buy your argument that the timing there doesn't matter. Once code has been released and there are external dependencies on it, that's obviously an issue. Ahead of that, I feel like the issue is really more one of interest- everyone is very interested when code is about to go in, but once it's in, for whatever reason, the interest becomes much less to review and comment on it. I feel like that's a very long-standing issue as it relates to our 'beta' period because doing code review just simply isn't fun and the motivation is reduced once it's been committed. That makes me wonder about having "beta" review-fests (in fact, I feel like that may have been proposed before...), where commits are assigned out to be reviewed by someone other than the committer/author/original reviewer, as a way to motivate individuals to go review what has gone in before we get to release. That might help us ensure that more of the committed code *gets* another review before release and reduce the issues we have post-release. Just a thought. Thanks, Stephen
pgsql-hackers by date: