20:00:55 #startmeeting glance 20:00:56 Meeting started Thu Nov 14 20:00:55 2013 UTC and is due to finish in 60 minutes. The chair is markwash. Information about MeetBot at http://wiki.debian.org/MeetBot. 20:00:57 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 20:00:59 The meeting name has been set to 'glance' 20:01:13 so who all do we have here today? 20:01:23 \0/ 20:01:25 o/ 20:01:29 * nikhil . 20:01:38 hi 20:01:39 ameade is really excited to be here 20:01:46 hi 20:02:02 \o 20:02:06 esheffield_: nah, i was just stretching 20:02:13 o/ 20:02:26 twoputt: flwang: ? 20:02:37 (just thought I'd highlight them in case they're around) 20:02:42 okay let's get started! 20:03:04 #topic Summit Review! 20:03:05 zhiyan: morning ;) 20:03:18 dosaboy: :) hey 20:03:27 we had all of our sessions on Tuesday 20:03:33 w00t 20:04:00 and honestly I have been working on the summaries sporadically since then 20:04:06 and so far I only need to finish up the taskflow one 20:04:10 then I'll send it all out the list 20:04:30 awesome! 20:04:40 the problem has been that summarizing is very thought provoking, so I end up sitting there thinking instead of typing 20:04:55 hahahaha, indeed it is 20:05:13 I think I spent the rest of the summit and my flight back having ideas of things to do 20:06:10 zhiyan: I think you added the note about figuring out what we're focusing on for this release, are there any notes you want to make on that front? 20:06:58 markwash: i have a draft list for myself. 20:07:30 but i'd like to see your list first. 20:07:51 there are a few ideas that have come up that weren't in our discussions, but were suggested by them 20:08:00 I suppose those ought to be blueprints as well 20:08:27 okay, well I'll get that sent out today to the ML, please read and respond in case I'm still missing something that was going on in the talks 20:08:32 next topic 20:08:45 #topic Glance Mini Summit (Tentative) Plans 20:09:09 Today I'll be sending out another email seeking feedback and interest in a Glance mid-cycle meetup / mini summit 20:09:41 wooooooooohoooooooooooooo!!!! 20:09:46 tentatively, the plan would be sometime in late January, and we would probably hold it in some small conference space in the Washington DC area 20:09:50 in hawaii right? 20:09:51 wooo 20:09:55 aww 20:09:57 +1 20:10:10 not sure how well that works for folks. . . but be sure to provide feedback in your responses 20:10:24 * nikhil quickly checks airbnb for some rental options 20:10:56 any questions about the meetup? 20:11:12 add the link to the form here? 20:11:25 sure that would be great 20:11:48 #link https://docs.google.com/forms/d/11DjkNAzVAdtMCPrsLiyjA7ck33jnexmKqlqaCl5olO8/viewform 20:11:55 that's the form you can use to indicate your interest 20:12:03 again I'll be sending that out to everyone on the ML later 20:12:28 since hopefully we'll get some interest from Nova folks and people outside the core group (that comes to the meeting :-) ) 20:12:38 all right, moving on 20:12:41 awesome 20:12:47 #topic Review Responsiveness 20:13:26 yes, there are some trivial patches that have just been sitting out there 20:13:33 I'd like us to take some time today to discuss ways we can improve how we process and handle reviews 20:13:45 for some context, here are our recent reviewstats links 20:14:04 #link http://russellbryant.net/openstack-stats/glance-reviewers-30.txt 20:14:16 #link http://www.russellbryant.net/openstack-stats/glance-openreviews.html 20:14:55 yeah, I took a look at those today. I've been a very bad reviewer lately 20:14:59 i think those numbers are helpful if we don't take them too far. . its possible to go overboard with metrics 20:15:49 so, I think we need to dedicate some more time to review patch of new contributors as well and make sure they leave with enough information 20:16:17 I also think we need to have a plan for this development cycle and then focus a bit more on the patches that help moving that plan forward 20:16:44 we may want to put some process in place 20:16:44 that being said, we definitely need more reviews. At least, I have to dedicate more time to it 20:17:18 I think a plan makes sense. . and I think some of the numbers being a bit bad is actually just a long hangover from focusing on getting in some core priorities 20:17:51 markwash: do we want to do something like nova does? or reinstate formal review day assignments? 20:18:12 there are some changes going into reviewstats soon that should probably help us get a grip on about how many reviews are needed per day 20:18:25 I don't think a 'review process' is what we need, we just need to keep an eye on those stats and make sure we don't leave patches un-reviewed. 20:18:38 and that the reviews help moving Glance plan forward 20:18:57 ameade: maybe. . but gosh I kinda hope we can avoid it personally 20:19:17 lifeless said something at the summit to me that I thought was pretty great 20:19:25 reviewing should be like brushing your teeth 20:19:36 just a habit that you do 3 times every day 20:19:42 yeah 20:19:43 yeah 20:19:52 and it's very personal 20:19:57 I mean, the workflow 20:20:05 and the way people organize the queue 20:20:19 markwash: +1 20:20:20 flaper87: yeah I think organizing the queue is a big question 20:20:39 so on the one hand, I was wondering a lot about top vs bottom feeding in terms of what to review first 20:20:53 but both of those have some casualties as well 20:21:09 for example, Dirk Mueller's patches that recently landed 20:21:20 he had these huge cleanup patches 20:21:29 that kept catching rebase conflicts 20:21:39 since they weren't high priority, they didn't really get picked out at the top 20:21:48 but since he had to keep uploading to fix conflicts, it never fell to the bottom exactly either 20:21:58 In those cases, a straight ping is worth it 20:22:08 imho 20:22:10 how are any of these thoughts different that what we are doing now? 20:22:19 yes, and maybe we just need to encourage that behavior from submitters 20:23:02 that sounds terrible, do you mean pinging every time they have a patch sitting around? 20:23:11 I wonder if we just push extra hard to get our queue length smaller, it might be easier to just scan the list and see what is most important 20:23:30 ameade: only every time they've had a patch sitting around for 3-4 weeks with no reviews :-) 20:23:58 haha gotcha 20:24:35 ameade: well, for one difference, right now we're averaging about 1.5 reviews per day per active reviewer over the past 30 days 20:24:50 maybe that number needs to be 4 per day to shrink the queue for a while 20:25:29 as long as we review faster than they need it 20:25:40 yeah! 20:26:13 that makes me wonder if I should pester people for reviewing some else's patches 20:26:27 nikhil: hmm, thats a good point 20:26:30 and get some credit poits for it :) 20:26:40 so, we've never had a bad-reviewers reputation, AFAIK. This is good. However, since we've getting more contributors and new exiting things going on, we may want to start dedicating a bit more time to it 20:26:44 see this was all planned ;=) 20:27:18 markwash: do you think every patch should have a bug id or bp ? except oslo sync-up changes? i mean what's the rule to make sure if a patch is important or not, sometime i need a priority. 20:27:20 if we see some reviews that look +2 to us, but need another core and might soon catch a rebase conflict, it would be a good idea to ping other folks in #openstack-glance 20:28:01 zhiyan: yeah, I kinda do think we need that. but on the other hand if we don't have a good system for ensuring those bps/bugs are well written, it might not really help 20:28:10 I've seen a lot of silly bps for oneoff changes in nova 20:28:31 i keep seeing C&P for bug desc and commit msgs 20:28:50 C&P ? 20:28:56 copy and paste 20:29:04 gross 20:29:08 ameade: WTF ? 20:29:25 (plop) 20:29:33 lol 20:29:39 +1 to what ameade noticed 20:30:29 do we think we need more core reviewers? 20:30:58 nope 20:30:59 I don't think so, for now 20:31:56 I for one really wish gerrit would just incorporate the more sensible parts of reviewstats and use that for sorting :-/ 20:32:02 what do you guys do exactly to figure out what to review? 20:32:22 i usually look at what is sitting aroudn the longest without -1s 20:32:38 I often just drop to the bottom of the list and scan upwards 20:32:57 lately I've been having better luck treating the glance review email notifications as a todo list 20:33:10 but obviously that latter approach won't fix the backlog 20:33:22 monty taylor had a good email a month or two back about how he organizes reviews in gerrit 20:33:42 * flaper87 bookmarked those links 20:34:09 hmm, I should look at that again 20:34:21 got links? 20:34:32 i have a todo list, including: 1) review ping, 2) team-room auto notification. 3) which I think important 20:34:48 I usually start from the bottom and look for patches that fix bugs and or sound like important. Then I do the same for the rest if nothing pops-up 20:34:59 I pick a bunch of them 20:35:27 and stick to those as much as possible, in order to be able to give good feedback and not having to refresh my memory everytime I open the review 20:35:28 flaper87: please share those links with us 20:35:35 * flaper87 looking 20:35:55 why not have one core reviewer on call per week and set a target for that week 20:36:05 i got the links, i use them all the time..looking for the email online 20:36:20 I'm not sure if anyone else has this problem, but sometimes I look at a review and I"m just like "what? I have no clue what you're doing" but it doesn't really feel like a -1 territory 20:36:35 I wanted to point out that one thing ameade has done good job in general is to take a deeper look at unit/functional tests while reviewing and would like to encourage in general amogst everyone 20:36:47 hehe, i have same feeling 20:36:58 like maybe I'm just not fully switched to the context of the review 20:37:05 #link http://lists.openstack.org/pipermail/openstack-dev/2013-September/015705.html 20:37:11 there you go guys 20:37:18 so I'll spend time reading code but then at the end of it I have no comment, and no + or - to add 20:37:46 flaper87: nice 20:38:18 markwash: if what the patch does is not clear, I think a better commit message is worth it 20:38:38 markwash: I got confused the other day too, when you said not a -1 on the patch we'r reviewing for correcting the locals 20:38:45 also, a good thing to do is, if we spend some time figuring out what a piece of code does, we could drop a comment there for the next reviewer 20:38:56 and to double check whether what we figured out is correct or not 20:38:57 markwash, flaper87: yeah i like to think that if i get confused reading a patch it was for a reason and maybe it could be clearer 20:39:08 I guess you don't feel strongly about such MPs ? Would you like to encourage the same? 20:39:28 okay cool, souncds like I should be commenting about my confusion if I have any 20:40:05 nikhil: I guess I use +0 sometimes to avoid the stigma of a -1 20:40:20 I prefer -1 that 0 20:40:21 but do you think that leaves submitters more confused? 20:40:31 at least it shows somebody already took a look at it 20:40:41 yeah +1 to -1 20:40:50 it's a stock queuing problem 20:41:00 sorry, responding to ages back 20:41:04 ha I hoped I'd summon you! 20:41:23 if your review queue steady state isn't approximately empty, when a big burst does arrive, you'll be killed 20:41:30 markwash: I guess that might have been a bad example, however sometimes I've observed that submitters don't pay attentions to the review if it's not a -1 20:41:45 (bad example as we'r working with him real time) 20:41:59 nikhil: i think that's good feedback 20:41:59 you may be overwhelmed temporarily by a big burst but if there is built up capacity in the team it will go away fairly quickly 20:43:28 what other problems to people experience doing reviews? 20:43:49 I think so far, possibly with the exception of my reviews, we haven't had a big problem with review quality 20:44:01 but if we're making a push for more reviews, its an important thing to keep in mind 20:44:26 some time i think it will be better if we have a "global" list which ordered by importance, and reviewers can focus one those changes.. sometime our effort are little scattering.. 20:44:46 zhiyan: why? 20:44:46 yeah I wonder if there is a good place for that? 20:44:49 markwash: what happend to reviewday? 20:44:54 https://github.com/dprince/reviewday 20:45:10 ameade: http://status.openstack.org/reviews/ 20:45:21 the problem with treating reviews as a priority queue 20:45:27 ah there we go 20:45:29 is that it assumes there is code we don't want to land 20:45:34 yeah 20:45:36 starvation 20:45:38 -and don't want to even review. 20:45:58 the other day iccha, hemanth and I were thinking about pair reviewing 20:46:01 But this is false: we're a community, the reviews are something we do ourselves to ourselves to keep quality up 20:46:06 (could be virtual parining too) 20:46:10 and the burden comes from code we write 20:46:11 frankly, we just need to review more...doesn't matter what or when, it's all important 20:46:13 lifeless: because each reviewers has different review todo list, and some change only has one +2, but it need wait more time to get another one. and sometime, it need rebase manually when it get two +2. 20:46:27 I feel like sometimes it's hard to understand the basic underlying approach from single point of view 20:46:30 zhiyan: ok, so the word 'importance' is overloaded :) 20:46:48 however, it helps when you talk it through with someone else 20:46:49 zhiyan: I'm fully in favor of driving partially reviewed patches all the way to landed before reviewing additional patches 20:46:50 I think there is one good reason for sharing some priority, and that is just that it might help to have a few core folks working in concert to reduce the current backlog 20:47:05 hopefully a priority queue aspect goes away when the queue is small 20:47:08 from a queuing perspective that keeps the average time waiting for reviewers lower 20:47:22 which reduces rework (rebasing, catching up with changing apis etc) 20:47:31 ah okay makes sense 20:47:52 is there a good filter for just "this already has a +2 from somebody other than you" in gerrit? 20:48:04 I think so - see chris jone's little python script 20:48:10 there is 20:48:27 I think one of the Monty's queries do that 20:49:05 at this point, we've spent about 35 minutes with some really great discussion of this topic 20:49:27 so one thing that might work with todays gerrit is several different custom searches - one with other +2s, one with +1s, one un-reviewed, and finally check for -1's with the submitter dissenting 20:49:28 are there any 1 or 2 things we all want to do to try to improve over the next week? 20:49:39 * lifeless shuts up :) 20:49:48 lifeless: no worries, thanks for your suggestions! 20:50:33 how about pair reviewing thing? 20:50:44 it helped me last night reviewing a patch with zhiyan 20:51:04 it speeds up the process much faster (thought that was something I was working on) 20:51:09 that sounds like a fine suggestion 20:51:27 beer? 20:51:30 another I would have, is I know I want to try to review at least 4-5 patches a day at least until the queue goes down 20:51:33 s/thought/though/ 20:51:34 dosaboy: +1 20:51:39 dosaboy: beer speeds up reviews, for sure! 20:51:41 :D 20:51:48 +2 beer 20:51:59 nikhil: markwash, actually it since nikhil's one just on the top of my todo list :) 20:52:02 i think for me personally, i need to set aside large chunks of time for reviews 20:52:10 * flaper87 too 20:52:30 also, lets try to review patches already reviewed and then tackle the rest down 20:52:31 i'm so slow at reviewing a lot of times i only get half way through before i am doing something else 20:52:31 or even flush of +2s ;) 20:52:41 flaper87: +1 20:52:47 sometime, i hope have a -3 button, to ask committer never push it up :) 20:52:52 haha 20:52:57 lol undo 20:53:01 we do have some persistent reviewers 20:53:13 constantly restoring after -1 auto-abandon 20:53:17 zhiyan: isn't that what the "Do Not Merge" is for? 20:53:17 s/reviewers/submitters 20:53:41 dosaboy: nope, that'd be for 'Do not merge and don't, ever, do it again' 20:53:43 :D 20:53:49 dosaboy: yes, but author can push next PS also. i mean "blocking" this at here. 20:53:56 ah ok 20:54:08 flaper87: cool 20:54:10 -3 you have lost the internet 20:54:16 okay 20:54:16 :D 20:54:18 haha 20:54:57 okay, I guess its time for open discussion 20:55:10 dosaboy: you have some important change want to review, rigtht? 20:55:17 4 items, iirc 20:55:20 I'm going to try to just review more, without pushing it to heroic proportions 20:55:32 well, i have a few patches that have been around for up to two months 20:55:53 i kinda discussed them at ods but they evolve arounbd race conditon fixes for v1 api 20:56:03 markwash: before reviewing, could you release a new version of python-glanceclient to pypi ? 20:56:15 whcih would be supersceded eventaully by a taskflow/statemachine approach 20:56:30 #topic Open Discussion 20:56:37 so questio is, do we want these to go throught and if so can we please move forwards with them 20:56:41 rosmaita: gah yes I suppose so 20:56:50 dosaboy: hmm, I don't think taskflow will fix anything with v1 20:56:54 the two shall never touch 20:57:09 e.g. https://review.openstack.org/#/c/50457/ 20:57:23 markwash: yeah there is that too 20:57:33 i was not clear on what the outlook was 20:57:47 so i still think we should go wih these patches 20:58:05 rosmaita: +1000 20:58:57 i'm guessing you guys are gonna take a similar approach to what cinder are doing 20:59:07 dosaboy: sort of, not exactly 20:59:14 cinder already has a remote rpc worker 20:59:32 right 20:59:34 that is located close to some important held state 20:59:51 for us I think the remote worker will initially be optional 21:00:07 but also, we'll probably land a good bit of stuff before integrating with taskflow 21:00:14 depending on the path of least resistence 21:00:34 all right, thanks everybody 21:00:46 #endmeeting