Hi all. Starting to think about ways to open up the contribution
process a little bit, while maintaining a high level of code quality.
The purpose of splitting out modules originally into Core and Extras
was to create a distinction between "Ansible Supported Modules" and
"Community Supported Modules". As we grow, the set of modules in
Extras should grow more quickly than elsewhere. Which also means that
we'll definitely need some help maintaining these modules.
So here's a simple proposal. Feedback welcome.
* Build a community team of triagers -- people who know Ansible
modules well enough to assess PRs and judge whether they're good.
* Give this team of triagers authority to upvote Extras PRs.
* Two +1s from triagers, on any Extras PR, means that an Ansible
developer can merge that PR immediately.
There's a lot of stuff we *could* do, all the way from triaging to
simply providing direct commit access to a bunch of folks. I'm hoping
we can start with some small but impactful changes to test the waters
a bit.
Not just for extras, core clearly needs some love, too.
Actively encourage anyone to review a PR. Request a few lines
justifying a +1 in specific terms:
- How's is this like/unlike similar modules?
- What were the results of testing it yourself?
- What changes would be good to add post-merge?
Developers can ignore PR's until there's at least a couple of +1's
with accompanying text. Encourage PR authors to solicit reviews from
good people.
If the original author doesn't respond to the last comment within a
couple of weeks, close with "please reopen this after responding to
the comments on it."
Not just for extras, core clearly needs some love, too.
I agree, but the bar for core needs to be higher. We can use extras to
figure out what works well and what doesn't.
We can, of course, encourage these kinds of reviews in core too -- but
they wouldn't lead to an automatic merge, maybe.
Actively encourage anyone to review a PR. Request a few lines
justifying a +1 in specific terms:
- How's is this like/unlike similar modules?
- What were the results of testing it yourself?
- What changes would be good to add post-merge?
Developers can ignore PR's until there's at least a couple of +1's
with accompanying text. Encourage PR authors to solicit reviews from
good people.
The goal is definitely to encourage reviewing as a standard practice,
and to move well-reviewed PRs to the front of the queue in all cases.
If the original author doesn't respond to the last comment within a
couple of weeks, close with "please reopen this after responding to
the comments on it."
Doug
I'd have an "unlikely to get merged without a couple good reviews"
policy in lieu of automatic merge. Devs with write access remain the
final arbiters but have much less work to do. Just read a couple of
good capsule reviews before making a decision.
Formally delegating/authorizing community members would be a lot of
work and carry the risk of admitting unsuitable members and passing up
on excellent ones.
I would LOVE to see all tickets with reviews/testing by the community,
it would make our job a lot easier and get tickets merged much faster.
Aside from a few 'high use' modules, this does not happen, when it
does those PRs do get merged much faster.
Part of this modules-extra approach is trying to make this a more
common occurrence. We have a huge backlog and we are just looking for
ways to not let PRs rot and address the needs of the community. We are
happy to consider any ideas that will help us with this. Changing the
policy to 'not merge w/o any reviews' will definitely make MY life
easier, but I don't think this will help the project or the community.
When an Ansible dev merges a review, give the good capsule reviewers one point each. Tally these on a prominent scoreboard. Top reviewers for each release are immortalized in the release announcement.
The policy is better as "reviews will fast-track the merge" rather than "no reviews == no merge."
As the original author of an -extras module, it would be nice if I could
be notified of issues in "my" module; more generally to be able to say
I'd like emails about issues in some set of modules that I feel I might
be able to help with. I don't want to see every -extras issue, but
having to remember to scan github for new issues is, I observe,
something I'm not very good at.
I agree. github doesn't have an easy way to automatically cc people
on a subset of the repository though. So we'll have to work out some
procedure to do that manually.
Maybe we could leverage github Teams? Create a team with the name of
the module (or the next category up as in the documentation
categories) and then whenever we get a bug report, always \cc @ansible/team-module as part of triage. The module author is put in
the team automatically. Figure out some loose policy to add other
people as well.
The problem with Teams is that it's not granular enough -- afaict, you
can't assign more granular permissions than "the entire repo level".
Which means that people who are tracking extras still have to track
the whole repo and not just their own stuff.
Maybe it's possible to use your mail reader to filter by Subject,
though, depending -- would need to play with the idea.
We also need to discuss the idea of "extras modules creators being
able to fast-track a push." Theoretically, the creator of the module
should know best whether patches should be applied, right?
I really love the idea and your proposal! It is basically what I ask for few months ago. So I definitely would like to try this out on extras!
Maybe technically a few thoughts:
Along with the “author” field in the modules, I thought about adding a “maintainers” list to the modules (with the github account names and/or email addresses), starting with the 3rd party committers of the modules, giving them opt-out.
So any maintainer gets an email on issues, PRs and commits related to that module and can give +1/-1 on PRs. Maybe we can build something in gerrit style for automate the process using the github API for merging directly on 2x+1 of maintainers.
I really love the idea and your proposal! It is basically what I ask for few
months ago. So I definitely would like to try this out on extras!
Fortunately, I need to make no claim about being original.
Maybe technically a few thoughts:
Along with the "author" field in the modules, I thought about adding a
"maintainers" list to the modules (with the github account names and/or
email addresses), starting with the 3rd party committers of the modules,
giving them opt-out.
So any maintainer gets an email on issues, PRs and commits related to that
module and can give +1/-1 on PRs. Maybe we can build something in gerrit
style for automate the process using the github API for merging directly on
2x+1 of maintainers.
So if I understand, we basically we update this file:
This way, maintainers can chose to be notified by github (using comments and @resmo) or by email.
Step: After that anyone has to make a tool, which notifies the maintainers if the PR or issue is not from the maintainers. If a PR is from a maintainer, then merge the PR immediately after successful travis build.
I believe that I am a big -1 on adding maintainers to the source of a module for this purpose. This could easily get out of date, and bloated with information. And making a code change to add a new maintainer doesn’t sound reasonable.
I’d propose another external tool which would allow individuals to pick their subscriptions. And actually I just started expanding my previous project to try and support this.
So, how do you go about building a community team? And how to ensure the community team is on the same page as the Ansible team when it comes to how a module should work and if it should exist in the first place?
I'd be happy to participate, but then I'm sure that most people with an open PR will say the same
Thanks for submitting your pull request! We thrive on the
contributions from people like you.
Before we take a look at your pull request, we ask that you first
review two outstanding pull requests that have the blue "review"
label. You can find them easily by going to this url: ... Our PR
review guide is at ....
Once you have reviewed two pull requests, we will mark your submission
with a review label and put it on the fast track to merge.
We know it's a little extra work, but your effort will help to make
sure we keep a quality system for everyone. It'll also get what you
need folded in quicker!
There should be no strings attached with submitting a PR. Everyone is welcome to submit PRs. Bug reports, and test others. Do it if you want, don’t if you don’t. Its literally that easy.
What about the string that keeps your PR from ever getting merged in
because there's 203984293 outstanding pull requests and a non-infinite
number of Ansible devs?