Open Stack

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.

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.

> 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.

> 
> 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).

> 
> 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.
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).
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)