Re: [openstack-helm] would like to discuss review turnaround time
Hi, I must apologize for not responding to this sooner. I feel some irony in that I, and many of the core review team in OpenStack-Helm, never been that great at OpenStack mailing lists, but are very active on IRC. To address the email that Jay sent to start this thread I've attached some comments.
There are just some example patch sets currently under review stage.
1. https://review.openstack.org/#/c/603971/ >> this ps has been discussed for its contents and scope. Cloud you please add if there is anything else we need to do other than wrapping some of commit message?
Last feedback was from myself on the 15th Jan, two weeks before the date of the email thread that started this conversation - I really like the PS, and see it has great value for operators of OSH, however, we really need to gate this, as we do all other components in OpenStack-Helm infra. Once we have a path there I see no reason not to merge, as my overly happy +1 wf finger has shown over the last few weeks.
2. https://review.openstack.org/#/c/633456/ >> this is simple fix. how can we make core reviewer notice this patch set so that they can quickly view?
This was merged within 48 hours of the initial commit, so I hope had an acceptable turnaround. Please feel free to add a reviewer via gerrit to any PS you think would benefit from their eye.
3. https://review.openstack.org/#/c/625803/ >> we have been getting feedbacks and questions on this patch set, that has been good. but round-trip time for the recent comments takes a week or more. because of that delay (?), the owner of this patch set needed to rebase this one often. Will this kind of case be improved if author engages more on irc channel or via mailing list to get feedback rather than relying on gerrit reviews?
This PS has indeed suffered from review stagnation, though this also highlights that we need to rely on more than just the core reviewers to provide meaningful feedback on patchsets. This PS is quite tricky to digest, as it touches every chart in the repo, and has changed direction several times in its lifespan. Here I think both a gentle nudge via gerrit would provide some assistance. Moving forward I'll also include announcements of the meeting agenda, and links to the logs in a weekly email for the project, which should hopefully allow easier engagement for those that find IRC a less convenient communication mechanism. Cheers Pete On Tue, 19 Feb 2019 at 8:46 AM, Jaesuk Ahn <bluejay.ahn@gmail.com> wrote:
It has been several weeks without further feedback or response from openstack-helm project members. I really want to hear what others think, and start a discussion on how we can improve together.
On Thu, Jan 31, 2019 at 4:35 PM Jaesuk Ahn <bluejay.ahn@gmail.com> wrote:
Thank you for thoughtful reply.
I was able to quickly add my opinion on some of your feedback, not all. please see inline. I will get back with more thought and idea. pls note that we have big holiday next week (lunar new year holiday), therefore, it might take some time. :)
On Wed, Jan 30, 2019 at 9:26 PM Jean-Philippe Evrard <jean-philippe@evrard.me> wrote:
Hello,
Thank you for bringing that topic. Let me answer inline. Please note, this is my personal opinion. (No company or TC hat here. I realise that, as one of the TC members following the health of the osh project, this is a concerning mail, and I will report appropriately if further steps need to be taken).
On Wed, 2019-01-30 at 13:15 +0900, Jaesuk Ahn wrote:
Dear all,
There has been several patch sets getting sparse reviews. Since some of authors wrote these patch sets are difficult to join IRC meeting due to time and language constraints, I would like to pass some of their voice, and get more detail feedback from core reviewers and other devs via ML.
I fully understand core reviewers are quite busy and believe they are doing their best efforts. period!
We can only hope for best effort of everyone :) I have no doubt here. I also believe the team is very busy.
So here is my opinion: Any review is valuable. Core reviewers should not be the only ones to review patches The more people will review in all of the involved companies, the more they will get trusted in their reviews. That follows up with earned trust by the core reviewers, with eventually leads to becoming core reviewer.
This is a very good point. I really need to encourage developers to at least cross-review each other's patch set. I will discuss with other team members how we can achieve this, we might need to introduce "half-a-day review only" schedule. Once my team had tried to review more in general, however it failed because of very limited time allowed to do so. At least, we can try to cross-review each other on patch sets, and explicitly assign time to do so. THIS will be our important homework to do.
I believe we can make a difference by reviewing more, so that the existing core team could get extended. Just a highlight: at the moment, more than 90% of reviews are AT&T sponsored (counting independents working for at&t. See also https://www.stackalytics.com/?module=openstack-helm-group). That's very high.
I believe extending the core team geographically/with different companies is a solution for the listed pain points.
I really would like to have that as well, however, efforts and time to become a candidate with "good enough" history seems very difficult. Matching the level (or amount of works) with what the current core reviewers does is not an easy thing to achieve. Frankly speaking, motivating someone to put that much effort is also challenging, especially with their reluctance (hesitant?) to do so due to language and time barrier.
However, I sometimes feel that turnaround time for some of patch sets are really long. I would like to hear opinion from others and suggestions on how to improve this. It can be either/both something each patch set owner need to do more, or/and it could be something we as a openstack-helm project can improve. For instance, it could be influenced by time differences, lack of irc presence, or anything else. etc. I really would like to find out there are anything we can improve together.
I had the same impression myself: the turnaround time is big for a deployment project.
The problem is not simple, and here are a few explanations I could think of: 1) most core reviewers are from a single company, and emergencies in their company are most likely to get prioritized over the community work. That leaves some reviews pending. 2) most core reviewers are from the same timezone in US, which means, in the best case, an asian contributor will have to wait a full day before seeing his work merged. If a core reviewer doesn't review this on his day work due to an emergency, you're putting the turnaround to two days at best. 3) most core reviewers are working in the same location: it's maybe hard for them to scale the conversation from their internal habits to a community driven project. Communication is a very important part of a community, and if that doesn't work, it is _very_ concerning to me. We raised the points of lack of (IRC presence|reviews) in previous community meetings.
2-1) other active developers are on the opposite side of the earth, which make more difficult to sync with core reviewers. No one wanted, but it somehow creates an invisible barrier.
I do agree that "Communication" is a very important part of a community. Language and time differences are adding more difficulties on this as well. I am trying my best to be a good liaison, but never enough. There will be no clear solution. However, I will have a discussion again with team members to gather some ideas.
I would like to get any kind of advise on the following. - sometimes, it is really difficult to get core reviewers' comments or reviews. I routinely put the list of patch sets on irc meeting agenda, however, there still be a long turnaround time between comments. As a result, it usually takes a long time to process a patch set, does sometimes cause rebase as well.
I thank our testing system auto rebases a lot :) The bigger problem is when you're working on something which eventually conflicts with some AT&T work that was prioritized internally.
For that, I asked a clear list of what the priorities are. ( https://storyboard.openstack.org/#!/worklist/341 )
Anything outside that should IMO raise a little flag in our heads :)
But it's up to the core reviewers to work with this in focus, and to the PTL to give directions.
- Having said that, I would like to have any advise on what we need to do more, for instance, do we need to be in irc directly asking each patch set to core reviewers? do we need to put core reviewers' name when we push patch set? etc.
I believe that we should leverage IRC more for reviews. We are doing it in OSA, and it works fine. Of course core developers have their habits and a review dashboard, but fast/emergency reviews need to be socialized to get prioritized. There are other attempts in the community (like have a review priority in gerrit), but I am not entirely sold on bringing a technical solution to something that should be solved with more communication.
- Some of patch sets are being reviewed and merged quickly, and some of patch sets are not. I would like to know what makes this difference so that I can tell my developers how to do better job writing and communicating patch sets.
There are just some example patch sets currently under review stage.
1. https://review.openstack.org/#/c/603971/ >> this ps has been discussed for its contents and scope. Cloud you please add if there is anything else we need to do other than wrapping some of commit message?
2. https://review.openstack.org/#/c/633456/ >> this is simple fix. how can we make core reviewer notice this patch set so that they can quickly view?
3. https://review.openstack.org/#/c/625803/ >> we have been getting feedbacks and questions on this patch set, that has been good. but round-trip time for the recent comments takes a week or more. because of that delay (?), the owner of this patch set needed to rebase this one often. Will this kind of case be improved if author engages more on irc channel or via mailing list to get feedback rather than relying on gerrit reviews?
To me, the last one is more controversial than others (I don't believe we should give the opportunity to do that myself until we've done a security impact analysis). This change is also bigger than others, which is harder to both write and review. As far as I know, there was no spec that preceeded this work, so we couldn't discuss the approach before the code was written.
I don't mind not having specs for changes to be honest, but it makes sense to have one if the subject is more controversial/harder, because people will have a tendency to put hard job aside.
This review is the typical review that needs to be discussed in the community meeting, advocating for or against it until a decision is taken (merge or abandon).
I do agree on your analysis on this one. but, One thing the author really wanted to have was feedback, that can be either negative or positive. it could be something to ask to abandon, or rewrite. but lack of comments with a long turnaround time between comments (that means author waits days and weeks to see any additional comments) was the problem. It felt like somewhat abandoned without any strong reason.
Frankly speaking, I don't know if this is a real issue or just way it is. I just want to pass some of voice from our developers, and really would like to hear what others think and find a better way to communicate.
It doesn't matter if "it's a real issue" or "just the way it is". If there is a feeling of burden/pain, we should tackle the issue.
So, yes, it's very important to raise the issue you feel! If you don't do it, nothing will change, the morale of developers will fall, and the health of the project will suffer. Transparency is key here.
Thanks for voicing your opinion.
Thanks you.
I would say my key take-aways are: 1) We need to review more 2) We need to communicate/socialize more on patchsets and issues. Let's be more active on IRC outside meetings.
Just one small note here: developers in my team prefer email communication sometime, where they can have time to think how to write their opinion on English.
3) The priority list need to be updated to be accurate. I am not sure this list is complete (there is no mention of docs image building there).
I really want this happen. Things are often suddenly showed up on patch set and merged. It is a bit difficult to follow what is exactly happening on openstack-helm community. Of course, this required everyone's efforts.
4) We need to extend the core team in different geographical regions and companies as soon as possible
But of course it's only my analysis. I would be happy to see Pete answer here.
Regards, Jeam-Philippe Evrard (evrardjp)
A bit unrelated with topic, but I really want to say this. I DO REALLY appreciate openstack-helm community's effort to accept non-English documents as official one. (although it is slowly progressing ^^) I think this move is real diversity effort than any other move (recognizing there is a good value community need to bring in "as-is", even though that is non-English information)
Cheers,
-- Jaesuk Ahn, Ph.D. Software R&D Center, SK Telecom
-- Jaesuk Ahn, Ph.D. Software R&D Center, SK Telecom
Hello, These were just examples. We can always find give counter examples of reviews lagging behind by checking in gerrit. As far as I understand it, the problem is not that people don't get reviews in a timely fashion, is how they are prioritized. In Openstack, reviews stays behind for a certain time. It's sad but normal. But when people are actively pointing to a review, don't get a review, and others patches seem to go through the system... Then a tension appears. That tension is caused by different understanding of priorities. I raised that in the past. I think this is a problem we should address -- We don't want to alienate community members. We want to bring them in, by listening to their use case and work all together. For this project to become a community project, we truly need to scale that common sense of priorities all together, far beyond the walls of a single company. We also need to help others achieve their goals, as long as they are truly beneficial for the project in the long term. I like this guidance of the TC: https://governance.openstack.org/tc/reference/principles.html#openstack-firs... . Let's get there together. Long story short, I would love to see more attention to non AT&T patches. I guess IRC, ML, or meetings are good channels to raise those. Would that be a correct assumption for the future? Anyway, I am glad you've answered on this email. Please keep in mind it's not possible for everyone to join IRC, some people are not fluent in English, and some people simply don't want to join instant messaging. For those, the official communication channel is also the emails, so we should, IMO, treat it as an important channel too. Regards, Jean-Philippe Evrard (evrardjp)
Sorry I'm late to the party. I won't disagree that we have room to grow here; in fact, I'd love to see our review throughput increase, both in terms of the number of reviews we're able to perform and the number of people performing them across the board. I also won't disagree that there have been times where review turnaround has been slower than others; however, I'd also like to point out that the response to reviews (whether it's further discussion or addressing review feedback) has also been slower at times than others. I'm not arguing that either one of those is more frustrating than the other, but in my mind it's an exercise in building trust. I'm always happy to review changes that come in that I feel qualified in reviewing, and I'm not above saying "today's a bit busier than usual, so it may be later before I can take a gander" -- we all have day jobs that demand our attention, even if those work periods don't line up across time zones. I don't think it's unreasonable to ask individuals desiring a quicker turnaround time on reviews for a quicker turnaround time on the feedback provided, else we don't really move forward here. I don't think solving the perceived priority and review latency issues is an insurmountable problem. The first step to tackling this is continued involvement in the channels mentioned above. Our weekly meeting attendance has fluctuated in the past, and I'm sure we can attribute some of that to the time zone difference; however, it's been pretty sparse recently. While our mailing list involvement may not be the best (it's easy to lose track of emails, personally), we've always had a standing portion of our weekly meeting devoted to posting changes we'd like reviewed. One of the items brought up recently was ensuring the following meeting's etherpad is posted at the end of the current weekly meeting so we've got ample time to get visibility on those. Furthermore, our weekly meetings are where we tend to discuss what's happening with OpenStack-Helm at any given time. Whether it's expanding what our jobs do, talking about what's required for getting new features in, or determining what someone can work on to add value, our weekly meeting would be the place to attend (or at least check out the meeting log if you can't make it). Our IRC channel is another -- I see plenty of chatter happening most days, so others needn't be shy about specifically calling out for reviews in the channel. I'm certain there are individuals who can't or don't want to be in IRC for reasons of their choosing, and that's fine - I personally plan to be more involved with the mailing list, but I also can't and don't want to be answering emails all day either. Once again, it's give and take. I'd challenge everyone in this thread to be part of the solution. We want to bring others in and accommodate their needs and uses, we want to grow a more diverse core team, and I personally enjoy talking to strangers on the internet. However, that requires active, continuous involvement from all parties involved, not just the core reviewer team - we're not the only ones who are capable of reviewing changes. I'm sure there's a path forward here, so let's find it together. Steve On Thu, Feb 21, 2019 at 12:28 PM Jean-Philippe Evrard < jean-philippe@evrard.me> wrote:
Hello,
These were just examples. We can always find give counter examples of reviews lagging behind by checking in gerrit.
As far as I understand it, the problem is not that people don't get reviews in a timely fashion, is how they are prioritized. In Openstack, reviews stays behind for a certain time. It's sad but normal.
But when people are actively pointing to a review, don't get a review, and others patches seem to go through the system... Then a tension appears. That tension is caused by different understanding of priorities. I raised that in the past.
I think this is a problem we should address -- We don't want to alienate community members. We want to bring them in, by listening to their use case and work all together. For this project to become a community project, we truly need to scale that common sense of priorities all together, far beyond the walls of a single company. We also need to help others achieve their goals, as long as they are truly beneficial for the project in the long term. I like this guidance of the TC: https://governance.openstack.org/tc/reference/principles.html#openstack-firs... . Let's get there together. Long story short, I would love to see more attention to non AT&T patches. I guess IRC, ML, or meetings are good channels to raise those. Would that be a correct assumption for the future?
Anyway, I am glad you've answered on this email.
Please keep in mind it's not possible for everyone to join IRC, some people are not fluent in English, and some people simply don't want to join instant messaging. For those, the official communication channel is also the emails, so we should, IMO, treat it as an important channel too.
Regards, Jean-Philippe Evrard (evrardjp)
participants (3)
-
Jean-Philippe Evrard
-
Pete Birley
-
Steve Wilkerson