(I am asking these questions to get a common understanding of how we
work as a community, not because I feel I am right or wrong)
I agree, people caring, and reviewing the code before/after and long
after commit is 100% what this community should be doing. No argument
there, what I want to know is how we achieve that... because at the
moment we have 2 processes for code review and its confusing.
So, neither I nor Ben pushed the patch to the appspot codereview. (not
least because my google id is not subscribed to shindig-dev and for
some reason I cant make aliases work properly with google id's)
but the patch was pushed into JIRA and then discussed at some length
on the list, ('Extension to getPerson for
0.9',SHINDIG-904,SHINDIG-918,'PersonService precludes filtering'),
starting on 5 Feb.
IMHO, that was review?
If patch + jira is not going to considered sufficient for code review
then IMHO we need to make that clear so decisions like the one I made
don't go un-noticed by those with production implementations until
they are found in production.
BTW, I was thinking of those who had already implemented the SPI in
making the decision, but obviously I was not privy to their source code.
I have no preference to how we work, either way is good with me, but I
don't want to be making decisions totally on my own, just because one
half of the community is working one way, and the other are working
another.
Ian
(also IMHO, just because something was reviewed doesn't mean that its
perfect and cant be made better in the future and always the case with
anything I write :))
On 20 Feb 2009, at 18:22, Adam Winer wrote:
As for the commit: In the past, we have reviewed patches in JIRA,
talked
about them and the committed them.
Have we changed the process to have all patches go into code review
before
being comitted ?
I dont remember seeing any conclusions about this on list?
No, there isn't any such requirement. Checking in without a review
means
there's a greater chance of someone griping after the commit, which
is a bit
more expensive for all concerned. That's what's happening now -
which is
all fine, and perfectly normal. The fact that people care is a Good
Thing
for the health of the project.
