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