[openstack-helm] would like to discuss review turnaround time

Jaesuk Ahn bluejay.ahn at gmail.com
Tue Feb 19 14:46:07 UTC 2019


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 at 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20190219/840c7c2a/attachment-0001.html>


More information about the openstack-discuss mailing list