On Sun, Jul 19, 2015 at 09:42:20PM -0400, Emilien Macchi wrote:
I'm currently in holidays but I could not resist to take some time and
reply.
Thanks for taking the time, apologies about distracting you from your
vacation!
Please read again my first comment on the Governance patch:
"They are making progress and I'm sure they are doing their best."
Ok, I'm happy to drop this topic and focus on the positive message here!
One way or the other, commits are a better way to measure progress than
emails :)
Andrew took the best metric in your advantage... the review metric,
which is something given to anyone having signed the CLA.
Review metric is just what Stackalytics shows by default, if Andrew were
shopping for the best metric he'd have used patch sets.
I would rather focus on patchset, commits, bug fix, IRC, ML etc... which
really show collaboration in a group.
And I honestly think it's making progress, even though the numbers.
Patch set count is naturally the first metric to show progress. Fuel
team's bugs triage process is already fairly mature, so I'm not
surprised there's noticeable contribution in that area, too.
Keeping that up and pushing for more IRC & ML participation should help
improve the patch set to commit ratio, which would make Fuel team more
productive and would improve the mutual trust, helping Puppet OpenStack
core reviewers trust that Fuel developers will not mess things up for
them, and helping Fuel developers trust that their commits will not get
stuck on review.
I think this should be our primary focus of improvement in the near
term. Once there's evidence that Fuel developers can produce quality
patches that can be landed relatively quickly, next level would be
improving our +1/-1 ratio and reviewing specs for blueprints.
[3] https://review.openstack.org/198119
(...)
Even before looking into the review comments, I could see a technical
reason for abandoning the commit: if there is a bug in a component,
fixing that bug in the package is preferrable to fixing it in puppet,
because it allows anybody to benefit from the fix, not just the
people deploying that package with puppet.
You are not providing official Ubuntu packaging, but your own packages
mainly used by Fuel, while Puppet OpenStack modules are widely used by
OpenStack community.
The only reason we've been doing our own packaging is lack of an
official cross-distro upstream. We've already opened access to all our
packaging code on review.fuel-infra.org, and want to move our packaging
work directly to upstream as soon as it is becomes possible:
http://lists.openstack.org/pipermail/openstack-dev/2015-July/069377.html
Fixing that bug in Fuel packaging is the shortest & easiest way for you
to fix that,
It's neither shortest nor easiest: it's 26 changed lines in 4 files, vs
4 changed lines in 2 files, and it was created only after we were told
in the puppet-horizon review that packaging is the right place to fix
it, so it was additional work on top of a workaround that we already
had.
while we are really doing something wrong in puppet-horizon
about the 'compress' option.
So Fuel is now fixed and puppet-horizon broken.
No, puppet-horizon simply lacks a workaround for broken horizon
packages, and that workaround also was reverted from fuel-library.
Mirantis may be the first to fix their packages, but if other distros
such as RDO or Ubuntu have the same problem in their packages, they
should fix it on their end, instead of relying on puppet, chef, ansible,
and other deployment tools to implement workarounds for their bugs.
It's a general "right tools for the job" problem. You can do a lot of
things with Puppet, but it doesn't mean that you should use Puppet for
everything. Managing options in a service's config files is Puppet's
job. Replacing package's init script, tweaking its directory structure,
creating symlinks, compiling binaries from code and so on should all be
done while you're building a package, not after you have installed it.
[4] https://review.openstack.org/190548
Here's what I see in this review:
a) Fuel team has spent more than a month (since June 11) on trying to
land this patch.
b) 2 out of 5 negative reviews are from Fuel team, proving that Fuel
developers are not "rubberstamping" each other's commits as was
implied by Emilien's comments on the TC review above.
c) There was one patch set that was pushed 3 business days after a negative
review, all other patch sets (11 total) were pushed no later than next day
after a negative review.
All in all, I see great commitment and effort on behalf of Fuel team,
eventually awarded by a +2 from Michael Chapman.
On the same day (June 30), Emilien votes -2 for a correction in a
comment, and walks away from the review for good. 18 days and 4 patch
sets and 2 outstanding +1's later, the review remains blocked by that
-2. Reaching out on #puppet-openstack [5] didn't help, either.
[5] http://irclog.perlgeek.de/puppet-openstack/2015-07-08#i_10867124
I -2 for this reason: this patch already had a +2 so it was closed to be
merged by someone else, while the patch was *breaking backward
compatiblity* in puppet-horizon, because Vasyl was changing the default
cache driver to force users to use Memcached by default.
We *never* break backward compatibility (this is a general rule in
OpenStack) and *never* change OpenStack default configuration (general
rule in Configuration management), but rather provide the interface
(called Puppet parameters) to change it.
That's fair enough, and on July 8 we pushed patch set 10 that has fixed
this problem, and set the default to LocMemCache. AFACT at this point
you've lost track of this commit because your gerrit dashboard excludes
all commits that have a -2 vote on them, so it got stuck in limbo for
two weeks.
I still disagree with your use of -2 vote here, a -1 from a core
reviewer should be enough to prevent other cores from merging a patch
set, and should tell them what to watch out for in subsequent patch
sets.
But anyway, adding a Disagreement section to the gerrit dashboard, and
going through the reviews that come up in it should help mitigate this
problem.
Fuel team can commit to reporting bugs and posting patches, Fuel team
can spend reasonable amount of time and effort to respond to review
comments, but all that is worth nothing if there's no commitment from
Puppet OpenStack team to review and merge these patches in a timely
manner.
Please consider our group is part of a few people comparing to Fuel's
team, and we are not all working 100% on Puppet OpenStack (I'm lucky, I
do). Please take that in consideration.
I understand that, and I know that most OpenStack projects are
struggling with keeping their review queue under control. As I've
mentioned, I understand that right now influx of patches from Fuel team
increases the burden on Puppet OpenStack core reviewers (both due to
core/non-core ratio, and due to patches/commits ratio). Hopefully it
won't be long before some Fuel developers gain enough knowledge and
reputation to start spreading out that additional burden.
We have weekly review/bug triage, feel free to participate, it happens
during our meetings.
We will, thanks.
Please don't get me wrong, but you're aggravating the situation.
I hope you meant "exagerrating" :)
We are having much more collaboration than before.
Fuel's team is much more involved on reviews as you highlighted, and
also on IRC, which is good.
Of course there are still technical things that will be improved with
more experience in working with Puppet OpenStack modules, but so far all
Puppet grouped noticed more collaboration I think.
I totally assume my -1 for adding Fuel to OpenStack because it's too
early now, even all your recent efforts.
Now, I think we just need you to let us more time, so your engineers and
our group can work together. I'm pretty sure the things will go
naturally like they happened for our other contributors.
Rather than doing yet another very long thread, maybe can we arrange a
public meeting on IRC and we can do some triage together. After my
holidays, I can even organize a 1 or 2 hour meeting so we can solve our
first blockers and make progress on both sides. I'll try to get
attention from our group if some help is needed.
What do you think?
I'll try to get a better attendance from Fuel contributors on the Puppet
OpenStack weekly meeting, hopefully that and gerrit dashboard tweaks
would be enough to improve the situation. I think we should do another
review of our collaboration around end of July, after two more weekly
meetings.
--
Dmitry Borodaenko
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
