18:00:07 #startmeeting trove 18:00:08 Meeting started Wed May 7 18:00:07 2014 UTC and is due to finish in 60 minutes. The chair is SlickNik. Information about MeetBot at http://wiki.debian.org/MeetBot. 18:00:09 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 18:00:11 The meeting name has been set to 'trove' 18:00:41 o/ 18:00:46 o/ 18:00:46 \0 18:00:52 o/ 18:00:52 \o 18:01:00 o/ 18:01:01 0/ 18:01:02 o/ 18:01:06 o/ 18:01:19 o/ 18:01:22 o/ 18:01:22 o/ 18:01:27 o/ 18:01:33 0/ 18:02:08 Agenda: 18:02:10 #link https://wiki.openstack.org/wiki/Meetings/TroveMeeting 18:02:27 o/ 18:02:42 Last meeting logs: 18:02:45 #link http://eavesdrop.openstack.org/meetings/trove/2014/trove.2014-04-30-18.02.html 18:03:01 o/ 18:03:16 #topic How do Gerrit changes get approved? 18:03:19 |o| 18:03:30 >o< 18:03:30 mat-lowery: take it away 18:03:36 o/ 18:03:36 i am curious about this too 18:03:37 :P 18:03:41 Goal: To clarify the Gerrit change approval process used by Trove core (for the benefit of core and non-core). 18:04:07 Should I just reproduce the text from the agenda? 18:04:20 All of my arguments are there. :) 18:04:38 That's not necessary. 18:04:47 o/ 18:05:10 I was giving this a lot of thought this week. 18:05:19 And a couple of things that made sense to me. 18:05:32 Does Trove core use a systematic process of reviews and approvals to prevent starvation? Ultimate goal: Reduce time between submittal and merge. 18:05:58 It's good idea to align on a process to prioritize and review changes 18:06:28 But I was looking at the reviewday tool 18:06:32 #link http://status.openstack.org/reviews/ 18:06:57 i'd like to be able to just see the reviews for the projects i care about in that review day tool 18:07:01 instead of EVERY project 18:07:12 project:openstack-trove in the search bar? 18:07:13 doesnt seem to have a anchor you can use either 18:07:14 I don't think there is a common tool / process that core uses. It might be good to pick one that we agree to 18:07:17 A lot of the top reviews there are actually waiting on changes from the submitter. 18:07:21 cp16net: You can run it locally but it takes forever. 18:07:36 oh… no more search bar 18:07:47 SlickNik: yea it's surprising so many with -1 and -2 show up at the top 18:07:55 kevinconway: this is an entirely differnt tool for reviews 18:08:33 ReviewDay uses a scoring algorithm based on Launchpad bug priority and age of Gerrit change. 18:08:35 review.openstack.org supports search customization. 18:08:37 i've often used: https://launchpad.net/trove/+milestone/juno-1 as a way to pick higher priority bugs/bps to focus on 18:08:40 my url is https://review.openstack.org/#/q/-label:CodeReview-2+AND+-label:+Verified-1+AND+status:open+AND+is:watched,n,z 18:09:35 that gives you just the projects taht you watch 18:09:40 and you can set in your lp profile 18:09:49 I think it might be good to review some of the review stats at every meeting to see how trove is doing . This is outdated, but if you look at http://russellbryant.net/openstack-stats/trove-reviewers-30.txt, it has some useful metrics like changes abandoned in the last 30 days, or queue growth in last 30 days 18:10:03 mat-lowery: Also a lot of reviews on reviewday that haven't yet passed unit-tests. 18:10:21 Doesn't make sense prioritizing some of those. 18:10:59 SlickNik, if you look at the URL I posted, it eliminates anything with a -2 or fails verified 18:11:00 http://www.stackalytics.com/report/reviews/trove-group/open 18:11:02 that may be what you want 18:11:07 SlickNik: Understood. I'm not selling ReviewDay but rather a prioritized queue (based on something that makes sense) that every core (and non-core) can/must use. 18:11:24 iccha1: Yes, the reviewstats tool hasn't been run since the gerrit upgrade. :( 18:11:33 mat-lowery: Got it. 18:11:34 i like the query amrith , thats something which used in glance too. 18:12:07 So sdague has a couple of good blog articles about this too 18:12:13 Also for those who want to customize their search, this page is helpful. #link https://review.openstack.org/Documentation/user-search.html 18:12:25 #link https://dague.net/2014/04/30/helpful-gerrit-queries-gerrit-2-8-edition/?utm_campaign=OpenStack+Now&utm_source=hs_email&utm_medium=email&utm_content=12695477&_hsenc=p2ANqtz-9twO0GbtB202VDA5wExXeBMIfQwnXdSBfuSj807F74UdfhZwJCsNKrVKI0pi-PqqRJQru8eyayQjntiU8sG1CaO4f1pA&_hsmi=12695477 18:12:44 My argument for a priority goes like this: If a priority exists, there's no need for "Hey core, please review " which I find inefficient and unfair. It seems that bugging the channel for reviews is OK to some. Why? 18:13:17 mat-lowery: well sometimes it is a blocking bug 18:13:28 mat-lowery: I find it okay because of a couple of reasons 18:13:32 so we should get eyes on it 18:13:36 robertmyers: Understood in those circumstances. 18:13:47 But otherwise, you're cutting the line. :) 18:13:59 is there necessarily a line? 18:14:01 Rarely do I prioritize a review above all others based on a poke 18:14:20 And it helps to get a conversation started. 18:14:25 :) 18:14:32 * esp visualizes a fight at walmart 18:14:32 I think as a team we would be more efficient with a priority list 18:14:47 right now our focus is like a disco ball rather than a magnifying glass 18:14:59 if everyone has the same pripirty list, wont the bottom ones be starved? 18:15:02 * amrith thinks about focusing on a disco ball 18:15:07 yeah i am in agreement we need to be better about it 18:15:10 juice: as we get more members and more patches, having a process with priority will help 18:15:24 +1 juice 18:15:33 iccha1: the ones at the bottom will not stay there forever 18:15:34 * robertmyers review are on top 18:15:39 iccha1 : right 18:15:40 iccha1: If the scoring is is age-influenced, then older (even low priority ones) rise to the top 18:15:44 they will actually move up because we cleared the ones the at the top 18:16:18 I think core should decide priority - we can use launchpad to set priority and all work from the same list 18:16:18 and priority should be last (or way down) if they dont pass jenkins 18:16:29 +1 18:16:46 there are some reviews where the bp has not been approved 18:16:52 I don't know if it is feasible to come up with search criteria that will do this for us 18:17:06 so the problem with just looking at patch age.. is that there are patches that exist for things that we have disagreement on 18:17:09 cp16net: then those reviews should have a low priority 18:17:10 priorities should be around - first go look at reviews you have already reviewed and folks has posted patchset on 18:17:13 cp16net: +1 18:17:19 vipul: +1 18:17:48 in fact we don't need to know the priority for all reviews 18:17:52 vipul, in that case, the core can -2 these patches bringing them down in priortiy 18:18:01 we just need to know which ones are the top priority (e.g. top 5) 18:18:10 the rest are unprioritized 18:18:13 i know some cores -2 patches when they need more clarity and discussions 18:18:25 esmute: It's not that easy. The patch might be a good idea, but it might be −1'ed to fix some issues. 18:18:33 In that case a −2 might be a bit harsh 18:18:34 esmute: that could work.. if we ignore -2's and factor in age 18:19:21 obviously there is isn't a simple answer here... needs a longer discussion 18:19:28 who's going to be in ATL!? 18:19:30 So the priority queue is liked in general. Just that the scoring isn't as Trove core would like? Is that fair? 18:19:30 so i kind of made the assumption that core would be looking for reviews that are +1 under "V" and had at least 1 +1 under "CR" 18:19:35 sounds like a good place to discuss this 18:20:09 dougshelley66: Thus my argument that -1's should not be left and then the reviewer disappears. 18:20:11 vipul, was question re: ATL targeted at core or all? 18:20:17 That's the next point: Gerrit etiquette. 18:20:22 anyone who wants to discuss this amrith 18:20:27 I'll be there 18:20:31 in ATL and to discuss this 18:20:35 dougshelley66: I know that we look for +1 from Jenkins (i.e. it has to pass the unit tests), but there's no requirement for it having a +1 under CR 18:21:02 SlickNik - i wasn't saying that was a requirement, just figured that would be a trigger 18:22:14 To wrap this item up, is core going to hammer out a scoring algorithm in ATL? 18:22:14 mat-lowery right; how to enforce the policy that you can't -1 and run 18:22:23 Another thing to keep in mind here is that we have some items that are just easy approves: Changes to help strings that make sense, changes to global-requirements, localization changes. 18:23:00 Perhaps number of +1's can bump the item up in score too aka easy approves. 18:23:55 #link https://wiki.openstack.org/wiki/ReviewWorkflowTips 18:24:06 looks like there are a number of tools available to help with this 18:24:19 My personal preference is to approve those sort of changes soon, so that it doesn't affect the size of the review pipeline. 18:25:09 I'll check out all of these review tip links but the point, as juice made was to have the queue be presented to you (no work from the reviewer about a query) then work from the top 18:25:24 mat-lowery, we cant enforce contributors to post only one type of marks if patch already has N +1's 18:25:35 vipul: oh nice 18:25:39 ok then I'll submit my patch with friends bumping it up wid their +1? 18:25:49 thanks for sharing those tools maybe that can help me 18:27:21 We're nitpicking a proposed algorithm. My point: At least have an algorithm. 18:27:27 And everyone follow it. 18:28:14 I will not be in ATL. Is there some kind of discussion list I should add this to? (And move on to next item.) 18:28:40 given that list of tooling that vipul provided, i assume this is a solved problem? wouldn't every project have the same issue? 18:28:58 I am in all for having a priority-based process for review like ReviewDay. We can keep making adjustment as we need (ie improve our scoring algorythm etc) 18:29:15 dougshelley66: i don't thnk it's a solved problem.. the tools may help pick reviews based on different criteria.. i am not sure if that's the criteria for us 18:29:36 we should still discuss what our priority queue is made up of 18:29:42 esmute: I hope we don't end up developing a review solution instead of a datastore service :) 18:29:54 SlickNik: weren't you going to propose a 'pod session' to talk about this 18:29:57 #action SlickNik to come up with a proposal on how to prioritize reviews to be discussed in ATL, and on IRC. 18:30:04 vipul: ok but i don't konw if i'm clear why all the projects couldn't tackle this issue in the same manner 18:30:05 Yes, I was. 18:30:06 but what about review neutrality? 18:30:20 if we allow service providers to create "fast-lanes" for reviews what next? 18:30:46 dougshelley66: good point.. i don't honestly know how other projects do it 18:31:25 May I move on to next point which is to discuss how all of us can keep (high-quality) code moving through the system? 18:31:50 vipul: we had the same problem in glance 18:31:55 vipul: Yes, I'm working on scheduling a pod session to discuss this issue in ATL. 18:31:55 so, can we proceed to next topic? 18:32:04 Stay tuned for more info regarding that. 18:32:23 mat-lowery: go for it. 18:32:25 SlickNik: mark wash from glance had tried to take some steps with the same 18:32:57 as dougshelley66 put it succinctly: you can't -1 and run. Do we all agree? There's an obligation to respond to follow-ups. 18:33:06 +1 18:33:10 +2 18:33:15 :) 18:33:34 mat-lowery, agreed, -1 without any reason seems invalid 18:33:34 +1 18:33:43 what if we have committment issues? 18:33:53 kevinconway: lol 18:34:08 mat-lowery: you can follow up with your reviewers right? 18:34:11 robertmyers: you are just encouraging him :) 18:34:18 denis_makogon: not without reason. leaving a -1...then the committer disagrees or needs clarification or whatever...and the original reviewer never returns 18:34:21 dougshelley66: +1 18:34:31 mat-lowery: what was the next point 18:34:43 dougshelley66: agreed 18:35:07 If running comprises not ever responding, then I agree with that sentiment. :) 18:35:34 mat-lowery, in this case, if noone followed same thoughts, -1 can be easily ignored 18:36:04 mat-lowery: my point from above is that at times if I don’t agree with a -1, I have to follow up with the review to clarify 18:36:06 But this is an async process, so I'd imagine that stuff can come up between when I make a comment and when the other involved party replies. 18:36:21 what do you expect from a -1 followup? 18:36:23 good one denis_makogon 18:36:28 pushing up new code get's rid of those 18:36:29 So if it takes me, maybe a day to reply to a comment in gerrit because of this, I think that's acceptable. 18:36:44 kevinbenton: :) that works too 18:37:00 sorry ^ kevinconway 18:37:08 kevinconway: I hope you're kidding. 18:37:21 But I have seen it. 18:37:25 Or what looks like it. 18:37:26 mat-lowery: +1 please dont' just push up a new patch set.. 18:37:32 at least reply to the comment 18:37:48 it's insane trying to figure out if all previous reviewer comments have been addressed 18:37:49 kevinconway: +1 :) 18:37:53 esp: Kevin bent on ? 18:37:58 mat-lowery: keeping clean code through the project I think was the next topic 18:38:08 SlickNik: I think we need metrics, how many reviews done by cores per day, non cores per day, how many patches get abandoned per day and why, etc. The question is are there enough reviews being done first, and then if yes are the right review sbeing looked at 18:38:09 I think we are on a tangent right now 18:38:09 SnowDust: yeah I didn’t get enough coffee.. 18:38:18 i'm not suggesting pushing up new code just to get rid of -1 votes 18:39:04 (high-quality) is what you said 18:39:05 i'm just curious why -1 is such a point of contention here 18:39:15 -1 is a community member saying "change this i don't like it" 18:39:23 btw, I've seen some behaviour in the new gerrit that doesn't get rid of the −1 even if a new patchset is pushed up. 18:39:38 I have too 18:39:40 but -1/+1 don't carry any great weight. you can agree or disagree. core makes the adult decisions 18:39:41 So this might be a moot issue, not that review.o.o has moved to the new gerrit version. 18:39:44 kevinconway: no ones is disagreeing with -1 - the disagreement is using a -1 without comments 18:39:45 now* 18:39:46 juice: My only point by adding high-quality was not to imply fast tracking code for the sake of speed. We can have good reviews and still get things merged. 18:40:32 i think this conversation is running in too many parallel directions :) 18:40:43 mat-lowery: sure we shouldn't sacrifice speed for quality is what you are saying 18:40:50 iccha1: +1 18:41:06 esp: +1 18:41:18 whoops - other way around in my last comment 18:41:18 SnowDust: -1 18:41:27 I don’t think anyone disagrees with the things being brought up 18:41:35 Okay, let me try to give this some definition 18:41:40 all: are we attempting to solve with a broad "policy" that which should be solved by individual interactions when a particular problem occurs? Is this a widely prevalent problem impacting a number of reviews? if yes, I see the point in the policy. If this is a fringe, how about core periodically do some coaching and address that way? 18:41:51 go for it slicknik - bring 'er to a close 18:42:47 I don't think anyone disagrees with any of the points mentioned in the Gerrit Ettiquette section. 18:43:00 #startvote No leaving -1s and then disappearing. A reviewer that leaves a -1 has an obligation to respond to follow up questions? yes, no 18:43:01 Begin voting on: No leaving -1s and then disappearing. A reviewer that leaves a -1 has an obligation to respond to follow up questions? Valid vote options are yes, no. 18:43:02 Vote using '#vote OPTION'. Only your last vote counts. 18:43:10 #vote yes 18:43:12 OK. Then if this is just a reminder of the etiquette, I'm happy 18:43:18 #vote yes 18:43:23 wait is this that you have to leave a comment or if you -1 you have to -1 every update? 18:43:23 #vote yes 18:43:24 #vote yes 18:43:25 #vote yes 18:43:27 kevinconway : waiting for an adult decision 4 ur -1 18:43:28 #vote yes! 18:43:28 vipul: yes! is not a valid option. Valid options are yes, no. 18:43:37 #vote yes 18:43:41 #vote yes 18:43:42 #vote yes 18:43:44 #vote yes 18:43:48 lol too excited about this one vipul 18:43:48 #vote yes 18:43:48 ? 18:43:52 #vote yes 18:43:58 #endvote 18:43:59 Voted on "No leaving -1s and then disappearing. A reviewer that leaves a -1 has an obligation to respond to follow up questions?" Results are 18:44:00 yes (11): juice, SlickNik, glucas, esp, esmute, amrith, denis_makogon, mat-lowery, cp16net, ramashri, dougshelley66 18:44:03 So just as I thought. 18:44:08 :) 18:44:14 i think this should be obvious and its a silly vote though. 18:44:16 lol, no way ) 18:44:18 -1 18:44:20 Are all the other ones similar, or is there a contentious issue? 18:44:28 cp16net: Yes, I feel the same way. 18:44:39 cp16net: humans by nature like winning 18:44:41 OK. So the lesson for me is to personally bug the people who are not following etiquette. 18:44:49 So another point is when we look at stackalytics Core team size: 5 (1.3 per core per day) reviews and Total reviewers: 31 (0.6 per reviewer per day). [http://www.stackalytics.com/report/contribution/trove-group/30] So maybe we need to just do more reviews too? 18:44:54 I tried Gerrit itself. No response. 18:44:55 !sftp 18:44:56 abramley: Error: "sftp" is not a valid command. 18:44:59 I'm not going to start a vote on them unless someone feels a dire need to talk about one of those rules. 18:45:02 Ooops - sorry 18:45:17 * amrith wonders why abramley is banging sftp 18:45:22 SlickNik: no need 18:45:28 iccha1: +1 I think that's the main issue 18:45:39 iccha1: size of core team and core team cycles spent on reviews 18:46:21 +1 18:46:23 more reviews off a prioritized list :) 18:46:43 and a lack of a prioritized list :p 18:46:43 iccha1: I think you bring up a good point here. 18:47:26 iccha1: We need to do a better job of tracking the stats, so that we know when we are slipping or if we need to grow core to cope with the review demand. 18:47:49 cinder has similar stats 18:47:53 actually even less 18:48:03 SlickNik: +1 and bring it up every meeting to keep tabs on the queue 18:48:34 iccha1: +1 18:48:44 iccha1: ugh. do we have to read previous meeting minutes into record too? 18:48:56 #action SlickNik to look at what stats to monitor for trove review health and include it in the aforementioned "review" proposal 18:49:27 I think we've beat this down for now. 18:49:33 i think thats a good idea to keep everyone in the know 18:49:37 +1 18:50:07 I'm going to put something together and will discuss with folks at ATL and on IRC. So stay tuned. 18:50:25 thanks SlickNik 18:50:32 sounds like a plan! 18:50:37 mat-lowery: Thanks for bringing it up! 18:50:45 #topic Meetings next week 18:51:00 So a lot of us are going to be in ATL next week for the summit. 18:51:14 yup 18:52:03 So I'm canceling the two Trove meetings for next week, since there will be a lot going on. 18:52:12 We'll meet again the week after that. 18:52:18 ok 18:52:18 sounds good 18:52:42 I'll send folks reminders on email / IRC. 18:52:57 I'll be on IRC on and off during the summit. 18:53:19 i'll be online all the time 18:53:28 doesnt mean i will respond tho 18:53:42 So if something comes up, feel free to message the trove channel. 18:53:58 That's all I had to say about that. :) 18:54:07 #topic Open Discussion 18:54:21 Any takers? 18:54:22 SlickNick ... Mid-Cycle Meetup Question 18:55:04 go for it 18:55:07 SlickNik, I'm trying to get a list of attendees for the mid-cycle meetup. if you'll be attending, please either drop me a note email or pm. 18:55:26 word 18:55:38 Sounds good. 18:55:43 amrith: is there a date for this? 18:55:57 aug 20-22 18:56:01 yes, let me check. dougshelley66 may know it off top of head 18:56:04 awesome thanks 18:56:05 yes, he does ;) 18:56:11 is there a wiki link with more info 18:56:12 ? 18:56:14 location: cambridge, MA 18:56:17 no wiki link yet 18:56:22 ok coo 18:56:23 good point 18:56:33 cp16net, coo to you too ;) 18:56:39 something like we had for the last one would be good 18:56:43 amrith: you might want to do the evite thing hub_cap did 18:56:55 that way whoever gets approved to go.. can just sign up 18:57:01 #action amrith make a wiki page for the midcycle meetup 18:57:12 cp16net, add one for eventbrite as well 18:57:17 #action that is 18:57:59 SlickNik, ... that's it for me ... 18:58:04 #action amrith to set up an eventbrite event for the mid-cycle meetup 18:58:11 amrith: if you have questions on how we planned the last one, let me know 18:58:12 (pro-tip: anyone can do it) 18:58:22 Sounds good. 18:58:25 Anyone else? 18:58:25 amytron, thx. was going to do that. 18:58:37 SlickNik: i made the PTL do it last time ;) 18:58:58 but you're right, anyone can do it :) 18:59:11 Okay, cool. 18:59:15 #endmeeting