Re: [gwt-contrib] Re: code review policy

2013-06-11 Thread Bhaskar Janakiraman
It would be good to formalize the process, and appoint 'maintainers' for 
different parts of GWT, who should be people who are knowledgeable, have 
key contributions, and are willing to take on that role. The advantages of 
doing this are:
1. Avoid bike-shedding.
2. Build up specialized knowledge, and
3. Get recognition for key contributors. 

Not having maintainers will cause trouble in the long term. Contributors 
should not fell compelled to respond to every comment if the people most 
experienced in that part of GWT have already given a +2. 

I think we should have a meeting and come up with a short list of people 
that we can nominate and see if they are willing to take on that role. 

Bhaskar



On Monday, June 10, 2013 2:16:22 PM UTC-7, Ray Cromwell wrote:

 I've been giving +2s for stuff I think is ready to commit, but I always 
 assumed that Googlers would have no superpowers here, and that Steering 
 Committee members and other designated people with specialized knowledge 
 could do so as well. Google doesn't automatically import changes without 
 reviewing them, and so our internal heuristic is something like if 
 importing changes from gerrit, if there isn't a +2 from a Google employee, 
 we need to take a deeper look. 

 For example, John giving +2 to I18N or OOPHM changes seems ideal to me. I 
 actually wouldn't feel comfortable giving more than a +1 on I18N unless 
 John was unavailable and it needed to get landed because of a critical 
 issue.



 On Mon, Jun 10, 2013 at 9:35 AM, Thomas Broyer t.br...@gmail.comjavascript:
  wrote:


 On Monday, June 10, 2013 5:12:34 AM UTC+2, Stephen Haberman wrote:

 Hey, 

 I wasn't quite sure, so figured I would ask: what's our policy on who 
 can give +2s? Is it any committer?


 Technically, yes: 
 https://gwt-review.googlesource.com/#/admin/projects/gwt,access (if you 
 can see it; restricted access)
 I don't know who exactly is a committer though. Of non-Googlers, at 
 least John (jat) and me.
  

 I assume so, but perhaps out of tradition, I've been assuming Googlers 
 would give +2s. 

 Well, and even if we are/have moved past the Google-only phase of GWT, 
 for now/awhile they will still have the most knowledge about the 
 codebase (perhaps sans Thomas).


 I try to not use +2 too much (or at least not submit reviews) because the 
 CI server currently doesn't run tests against real browsers, and doesn't 
 run tests at all pre submit.
 Maybe we should do something like OpenStack: 
 http://ci.openstack.org/gerrit.html They have a third category 
 Approved in addition to Verified and CodeReview, when set to 
 approved then the CI server is triggered to do a build, and updates the 
 Verified score. This is so that they can control the charge put on their CI 
 server (only code that looks OK is built; but it's not worth scrutinizing 
 the code if it'd break, so they don't wait the CodeReview+2).
 I think we could have our first checkstyle check done automatically for 
 all reviews (like today, when it works ;-) ) but only put Verified-1 if it 
 fails, and trigger a full build with tests only with an Approved+1 (or 
 CodeReview+2 if that's easier to setup). Ideally, the build could just run 
 JVM tests, and we (committers) could trigger browser tests (HtmlUnit only, 
 or in real browsers) if needed.
 But that's not easy to setup (and someone would have to do it), so for 
 now I submit sparingly (unless Googlers tell us to go ahead): for now, 
 tests are run by Google when master is merged on google/pu, and they do 
 reverts when something breaks.
  
 -- 
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 --- 
 You received this message because you are subscribed to the Google Groups 
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an 
 email to 
 google-web-toolkit-contributors+unsubscr...@googlegroups.comjavascript:
 .
 For more options, visit https://groups.google.com/groups/opt_out.
  
  




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [gwt-contrib] Re: code review policy

2013-06-10 Thread Ray Cromwell
I've been giving +2s for stuff I think is ready to commit, but I always
assumed that Googlers would have no superpowers here, and that Steering
Committee members and other designated people with specialized knowledge
could do so as well. Google doesn't automatically import changes without
reviewing them, and so our internal heuristic is something like if
importing changes from gerrit, if there isn't a +2 from a Google employee,
we need to take a deeper look.

For example, John giving +2 to I18N or OOPHM changes seems ideal to me. I
actually wouldn't feel comfortable giving more than a +1 on I18N unless
John was unavailable and it needed to get landed because of a critical
issue.



On Mon, Jun 10, 2013 at 9:35 AM, Thomas Broyer t.bro...@gmail.com wrote:


 On Monday, June 10, 2013 5:12:34 AM UTC+2, Stephen Haberman wrote:

 Hey,

 I wasn't quite sure, so figured I would ask: what's our policy on who
 can give +2s? Is it any committer?


 Technically, yes:
 https://gwt-review.googlesource.com/#/admin/projects/gwt,access (if you
 can see it; restricted access)
 I don't know who exactly is a committer though. Of non-Googlers, at
 least John (jat) and me.


 I assume so, but perhaps out of tradition, I've been assuming Googlers
 would give +2s.

 Well, and even if we are/have moved past the Google-only phase of GWT,
 for now/awhile they will still have the most knowledge about the
 codebase (perhaps sans Thomas).


 I try to not use +2 too much (or at least not submit reviews) because the
 CI server currently doesn't run tests against real browsers, and doesn't
 run tests at all pre submit.
 Maybe we should do something like OpenStack:
 http://ci.openstack.org/gerrit.html They have a third category Approved
 in addition to Verified and CodeReview, when set to approved then the
 CI server is triggered to do a build, and updates the Verified score. This
 is so that they can control the charge put on their CI server (only code
 that looks OK is built; but it's not worth scrutinizing the code if it'd
 break, so they don't wait the CodeReview+2).
 I think we could have our first checkstyle check done automatically for
 all reviews (like today, when it works ;-) ) but only put Verified-1 if it
 fails, and trigger a full build with tests only with an Approved+1 (or
 CodeReview+2 if that's easier to setup). Ideally, the build could just run
 JVM tests, and we (committers) could trigger browser tests (HtmlUnit only,
 or in real browsers) if needed.
 But that's not easy to setup (and someone would have to do it), so for now
 I submit sparingly (unless Googlers tell us to go ahead): for now, tests
 are run by Google when master is merged on google/pu, and they do reverts
 when something breaks.

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors
 ---
 You received this message because you are subscribed to the Google Groups
 GWT Contributors group.
 To unsubscribe from this group and stop receiving emails from it, send an
 email to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- 
You received this message because you are subscribed to the Google Groups GWT 
Contributors group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.