Hi fellow devs,
I've been thinking lately the project would benefit from having a few
more people with commit access into Foreman core for a couple of
reasons.
-
It motivates people to review more code. I gained commit access after
reviewing PRs and then pinging someone to merge, but it doesn't look
like others know this route or think it's interesting. It can be
demotivating to review PRs to never get the chance to say 'ok this goes
in'. Giving people the small 'prize' of merging PRs they review is also
a gentle nudge to encourage them to review and actually do it.
-
We are building a culture where people expect their PRs to core to
be reviewed by a very reduced group.
This leads to a lot of work for current reviewers, who don't get much
time to make other meaningful contributions to the project.
This kind of culture makes people churn out pull requests and then get
frustrated because they cannot get reviewed fast enough. Yesterday I
reviewed a PR from afeferku that was sitting for 27 days untouched.
-
It's only fair that if you have been contributing to a project for a
while, know what kind of standards are expected, you also get the right
to drive it forward.
-
I trust these people will not start by merging the ~85 PRs open right
away, but will instead start with easy stuff that's clear that should be
merged. I trust them as I trust other reviewers that if they're not sure
something should go in, they will ask other people to help.
···
---------------------------------------------------------------
shlomizadok, orabin, tbrisker, shimshtein, and unorthodoxgeek
have been making contributions to the project so I would like to give
them commit access and have them as involved as possible on the project
as other committers.
Best, please let me know your thoughts on this.
–
Daniel Lobato Garcia
@dLobatog
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: https://keybase.io/elobato
TLDR - Broadly, +1, but MOAR DOCS.
···
--
More specifically, there's a few issues wrapped up in this..
Firstly, you raise the issue of process - new contributors in the dev
community don’t really know what to expect or how to progress. Whatever we
decide in terms of new committers, we should document the process.
Something like
https://www.mozilla.org/en-US/about/governance/policies/commit/ (thanks to
Daniel for the link) tailored to our needs would suffice. Once we’ve had
the rest of this debate, I can send a PR for something like this to the
website.
Secondly, as you know, we’ve been discussing how to improve the number of
reviewers, and I think this is good discussion to have. I have no issue
with more committers - with the number of forks of Foreman and it’s various
repos, any malicious or accidental damage could be repaired in minutes. I
like the idea of recognition for reviewers (I still need to implement the
scoreboard idea from last month). I’m also liking the ‘vouch’ or 'sponsor’
system in the link above. We would need to decide how many are needed - I
can certainly see 2 or 3 for core repos, but maybe only 1 for plugins/side
stuff.
However, while I don’t want to set a minimum-number-of-reviews as part of
the commit process, I do think we need to see some review action from
potential committers. Reviewing is a different skill to coding, and needs
to be demonstrated. Being able to write solid code of your own as a
contributor is great, but there’s more to commit access than that.
The other key thing is that we need to ensure new committers understand our
rules around merging - I was just looking in the handbook, and I don’t
believe we’ve documented that we don’t merge our own PRs (individual repos
may have this in their own docs). With that understanding in place, some
"prior art" in reviews, and a sponsor, I think this is good.
Cheers
Greg
I would second Ori Rabin and Tomer Brisker, for the consistent quality
of their contributions, maintenance and reviews.
If they're interested and there aren't any objections, I'm happy to
add them some time next week.
Dominic Cleal
dominic@cleal.org
···
On 22/01/16 08:40, Daniel Lobato Garcia wrote:
> shlomizadok, orabin, tbrisker, shimshtein, and unorthodoxgeek
>
> have been making contributions to the project so I would like to
> give them commit access and have them as involved as possible on
> the project as other committers.
> However, while I don't want to set a minimum-number-of-reviews as part of
> the commit process, I do think we need to see some review action from
> potential committers. Reviewing is a different skill to coding, and needs
> to be demonstrated. Being able to write solid code of your own as a
> contributor is great, but there's more to commit access than that.
Broadly speaking I agree, the only problem I have with that is some people
won't consider reviewing a part of their duty as a contributor (consciously or
inconciously) unless they can both review and commit. I'd say that was the
case for orrabin & stbenjam on Discovery, obviously this is just a personal
perception.
···
--
Daniel Lobato Garcia
@dLobatog
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: https://keybase.io/elobato
Hello,
Thank you Daniel & Dominic for your vote of confidence.
I'd be happy to join you as a commiter and try to take at least some of the
load off your shoulders.
I agree with Greg that the docs could be updated with more relevant info.
I hear David's pain with the lack of commiters in the western hemisphere,
unfortunately I won't be able to fix that as I'm located in Israel.
I will however be able to commit on Sundays and Christian holidays, when
most of the western world is not working.
Have a nice day,
Tomer
···
On Fri, Jan 22, 2016 at 4:21 PM, Dominic Cleal wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 22/01/16 08:40, Daniel Lobato Garcia wrote:
shlomizadok, orabin, tbrisker, shimshtein, and unorthodoxgeek
have been making contributions to the project so I would like to
give them commit access and have them as involved as possible on
the project as other committers.
I would second Ori Rabin and Tomer Brisker, for the consistent quality
of their contributions, maintenance and reviews.
If they’re interested and there aren’t any objections, I’m happy to
add them some time next week.
Dominic Cleal
dominic@cleal.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iEYEARECAAYFAlaiOvUACgkQfH0ybywrcsy7NACfQNbQGFqhPF/Cev9Yu5GLT492
m9MAoLe4lTlhUnpOJoxm0hLB8GWWp7nM
=sVAP
-----END PGP SIGNATURE-----
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
–
Have a nice day,
Tomer Brisker
Red Hat Engineering
> From: "Daniel Lobato Garcia" <elobatocs@gmail.com>
> To: foreman-dev@googlegroups.com
> Sent: Friday, January 22, 2016 11:27:15 AM
> Subject: Re: Re: [foreman-dev] Vouching to extend commit rights for Foreman core
>
> > However, while I don't want to set a minimum-number-of-reviews as part of
> > the commit process, I do think we need to see some review action from
> > potential committers. Reviewing is a different skill to coding, and needs
> > to be demonstrated. Being able to write solid code of your own as a
> > contributor is great, but there's more to commit access than that.
>
> Broadly speaking I agree, the only problem I have with that is some people
> won't consider reviewing a part of their duty as a contributor (consciously
> or
> inconciously) unless they can both review and commit. I'd say that was the
> case for orrabin & stbenjam on Discovery, obviously this is just a personal
> perception.
I'd gotten commit as part of joining a discovery feature team, same for remote
execution. I'd never have just randomly started reviewing discovery PR's.
Occasionally something comes up in core in an area I'm familiar with, and I just
happen to see it. But I think that's not very often that I notice, because I'm off
doing other things. I thought this is one of the things that having a "core" team
to begin with? Having people to maintain the core projects and infrastructure and
letting people working on features dedicate their time to it.
If they don't have the bandwidth, perhaps the team should be bigger, and I certainly
agree giving commit to the people mentioned by Daniel is a good idea.
Although, so far I think everyone on the core team is only Red Hat, the concept should
probably be formalized in the community. Having contributors review PR's should
certainly be an area for contribution, and a path to joining the core team, but
that isn't even on Foreman :: Contribute.
···
----- Original Message -----
–
Daniel Lobato Garcia
@dLobatog
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: https://keybase.io/elobato
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
A pain point for me in particular is having all the people with commit
rights to foreman core IN ONE SINGLE TIMEZONE/REGION. This never made sense
to me seeing as how we have contributors outside that timezone. We’ve been
blocked tons of times like when gems started dropping Ruby 1.9 support and
Jenkins failed. When this happens after 12pm EST, there’s often no one
around to merge the PR that can fix this problem and unblock us. So more
people with commit access is nice if it means we can get at least one
person outside the Europe/Israel timezones.
David
···
On Fri, Jan 22, 2016 at 7:17 AM, Stephen Benjamin wrote:
----- Original Message -----
From: “Daniel Lobato Garcia” elobatocs@gmail.com
To: foreman-dev@googlegroups.com
Sent: Friday, January 22, 2016 11:27:15 AM
Subject: Re: Re: [foreman-dev] Vouching to extend commit rights for
Foreman core
However, while I don’t want to set a minimum-number-of-reviews as part
of
the commit process, I do think we need to see some review action from
potential committers. Reviewing is a different skill to coding, and
needs
to be demonstrated. Being able to write solid code of your own as a
contributor is great, but there’s more to commit access than that.
Broadly speaking I agree, the only problem I have with that is some
people
won’t consider reviewing a part of their duty as a contributor
(consciously
or
inconciously) unless they can both review and commit. I’d say that was
the
case for orrabin & stbenjam on Discovery, obviously this is just a
personal
perception.
I’d gotten commit as part of joining a discovery feature team, same for
remote
execution. I’d never have just randomly started reviewing discovery PR’s.
Occasionally something comes up in core in an area I’m familiar with, and
I just
happen to see it. But I think that’s not very often that I notice,
because I’m off
doing other things. I thought this is one of the things that having a
"core" team
to begin with? Having people to maintain the core projects and
infrastructure and
letting people working on features dedicate their time to it.
If they don’t have the bandwidth, perhaps the team should be bigger, and I
certainly
agree giving commit to the people mentioned by Daniel is a good idea.
Although, so far I think everyone on the core team is only Red Hat, the
concept should
probably be formalized in the community. Having contributors review PR’s
should
certainly be an area for contribution, and a path to joining the core
team, but
that isn’t even on Foreman :: Contribute.
–
Daniel Lobato Garcia
@dLobatog
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: https://keybase.io/elobato
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
True - and not something we can resolve. They represent two sides of the
same coin (reviewing before or after getting commit), and we must pick a
side 
···
On 22 January 2016 at 10:27, Daniel Lobato Garcia wrote:
However, while I don’t want to set a minimum-number-of-reviews as part of
the commit process, I do think we need to see some review action from
potential committers. Reviewing is a different skill to coding, and needs
to be demonstrated. Being able to write solid code of your own as a
contributor is great, but there’s more to commit access than that.
Broadly speaking I agree, the only problem I have with that is some people
won’t consider reviewing a part of their duty as a contributor
(consciously or
inconciously) unless they can both review and commit. I’d say that was the
case for orrabin & stbenjam on Discovery, obviously this is just a personal
perception.
Ori and Tomer - Welcome to foreman core maintainers team - congratulations!
Ohad
