Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

[Vote ended on 2022-06-03] What to do with a lack of collection inclusion reviews? #97

Closed
Andersson007 opened this issue Apr 28, 2022 · 16 comments

Comments

@Andersson007
Copy link
Contributor

Andersson007 commented Apr 28, 2022

Summary

Context

According to the Steering Committee guidelines, the Committee members review
collections submitted for inclusion in the Ansible community package.

The inclusion discussions are happening in the ansible-inclusion repository.

At the moment, a collection to be included in the package MUST be approved by, at least, TWO Committee members.

Problem to discuss and solve
  • We have a lack of reviews, therefore, the submitted collection can't get into the package.

  • We have a big inclusion backlog: new collections are submitted, old ones stay there.

How the process works now

In a nutshell, the process now consists of the following steps:

  1. A Steering Committee member copies the Collection checklist that reflects the Community collection requirements to a discussion.
  2. The member goes through the checklist checking if the collection satisfies the requirements:
  • If it satisfies a condition, the member marks the corresponding checkbox complete.
  • If it does not satisfy, the member add a comment under the condition starting with MUST FIX.
  • If there are something that can be improved, the comment starts with SHOULD FIX.
  1. After the collection maintainers fix all MUST FIX points and report about it:
  • The member checks them.
  • If there are no other concerns, the member explicitly approves the inclusion.
Possible solutions to the lack of reviews problem
  1. IMO, to fix the situation, the current workflow must be redesigned OR see option b below.
  2. (Your solution here)

Possible implementation options:
a) Change the inclusion requirements: for a collection to be included it's enough to have 1 Steering Committee member review and 1 Community member review (instead of 2 Committee reviews as is now). The Community member must not be involved in the collection. This can potentially increase the number of volunteers but with no guarantee.
b) To expand the Steering Committee Chairperson's responsibilities to give them power to assign Committee members to review the collection.

  • We create a list of members who have experience in collection/module development/maintenance.
  • Volunteers are welcome (@Andersson007 will often be one of them). Before assigning, the chairperson (or another member) will call for volunteers.
  • If there are two volunteers, no other action is needed.
  • If there are one or no volunteers, the chairperson will assign the inclusion request trying to be fair (one-by-one) and considering a person's circumstances (e.g. busy time at day job, PTO, family reasons, etc).

c) (your option here)

Please share your thoughts, it's really a problem now.

@MarkusTeufelberger
Copy link

Some (a lot?) of these checks can be automated. It sounds very tedious to have 2 humans look and mark checkboxes if a README.md file exists for example.

@felixfontein
Copy link
Contributor

That part's also related to #15. Some of the things are relatively easy to check automatically. Quite a few are not, though, and these are usually the more interesting things.

@Andersson007
Copy link
Contributor Author

Quite a few are not, though, and these are usually the more interesting things.

Yep, reviewing by at least two persons is imo important as this (i think almost always) happened that the second reviewer spotted things that the first one had missed (things that is hard to automate like adhering the dev conventions).
Automation would definitely be very helpful, though it won't fix the main issue.

@mariolenz
Copy link
Contributor

I'm quite new to the steering committee, so I don't really feel qualified to discuss this. But one thing I can say:

Yep, reviewing by at least two persons is imo important as this (i think almost always) happened that the second reviewer spotted things that the first one had missed

I fully agree. Especially since the first reviewer probably already had lots of looks at the collection. In my experience, if someone didn't spot a problem at the first couple of times, they won't at the hundreth time, either. At least, that's what happens to me all of the time.

One other thing: I've had a look at ansible-inclusion and I don't find the categories really helpful. For example, under New collection reviews there are really new inclusion requests that nobody had had at look at yet, but also some that already had dozens of comments. Maybe there should be more categories, like First review needed, First review in progress, Second review needed and Second review in progress. At the moment, I wouldn't even know where to start.

If this doesn't make sense, just ignore me. As I've already said, I'm quite new to the steering committee.

@Andersson007
Copy link
Contributor Author

Andersson007 commented May 3, 2022

@mariolenz

If this doesn't make sense

I believe it does:) Good idea, thank you!
Should be very helpful and make (potential) reviewer's life much easier.
I'll add the categories.

Update: I added more categories and sorted out the discussions. So there are the following ones:

  1. New collection reviews
  2. First review in progress
  3. Second review needed
  4. Second review in progress
  5. In process of inclusion
  6. Resolved reviews

@Andersson007
Copy link
Contributor Author

i suggest relaxing the requirements with the following (it's not final wording, only meaning):

  • One Steering committee review and approval and one non-Steering committee person's (not involved in the collection) review and approval is a requirement for the collection to be included in the package.

We could advertise this via Bullhorn among the community (in particular, among collection contributors and maintainers) as a good way:

  • to contribute to the package enhancement
  • to improve skills
  • to get into the Steering Committee

We could announce new inclusion requests via Bullhorn regularly and it'll help:

  • Make the whole community involved in the process
  • Inform the community about new collections that can be included in the future

What do you think? If there's silence (i.e. no major objections), I'll create a PR and will open a vote. Meanwhile, the backlog of submitted collections is growing..

@felixfontein
Copy link
Contributor

I don't think that relaxing the requirements is a good solution for this problem. I definitely would like more community interaction (which also means: reviews by the community), but I think every community review should also be checked by at least one ST person whether the review and the changes asked for makes sense. And if one review is by a non-ST person, I think it's still reasonable to expect someone from ST to take a quick look at the collection for obvious problems that were missed in the reviews.

@Andersson007
Copy link
Contributor Author

Andersson007 commented May 19, 2022

I don't think that relaxing the requirements is a good solution for this problem. I definitely would like more community interaction (which also means: reviews by the community), but I think every community review should also be checked by at least one ST person whether the review and the changes asked for makes sense. And if one review is by a non-ST person, I think it's still reasonable to expect someone from ST to take a quick look at the collection for obvious problems that were missed in the reviews.

Sounds sensible to me, so how about (this is a very rough text):

  • At least one Steering committee review and approval and one non-Steering committee person's (not involved in the collection) review and approval (checked and approved by another SC member) is a requirement for the collection to be included in the package.

So there will be still 2 SC members involved but there will be less work for them. Thoughts?

@Andersson007
Copy link
Contributor Author

I created a PR ansible/ansible#77905. Let's polish it a bit, so please take a look.
If there are no objections, i'm gonna open a vote in a couple of days.

@Andersson007
Copy link
Contributor Author

Andersson007 commented May 27, 2022

The vote on merging the PR has been just opened.

As an experiment described in another topic, I've created the vote dicsussion.
Please vote in the discussion (with +1/-1), NOT here.

To avoid the noise caused by voting, subscribe only for issues:

  1. Click the down arrow on the Watch/Unwatch button
  2. Choose Custom
  3. Choose at least Issues
  4. After voting in the discussion, click Unsubscribe in the discussion

The vote will end on June 3, 2022

@Andersson007 Andersson007 changed the title What to do with a lack of collection inclusion reviews? [Vote ends on 2022-06-03] What to do with a lack of collection inclusion reviews? May 27, 2022
@Andersson007 Andersson007 added active-vote These are currently active votes and removed discussion_topic labels May 27, 2022
@mariolenz
Copy link
Contributor

  1. Click the down arrow on the Watch/Unwatch button
  2. Choose Custom
  3. Choose at least Issues
  4. After voting in the discussion, click Unsubscribe in the discussion

Looks a bit complicated to me. Personally, I'd rather live with just ignoring +1 / -1 comments. Seems easier to me.

@briantist
Copy link

Looks a bit complicated to me. Personally, I'd rather live with just ignoring +1 / -1 comments. Seems easier to me.

To be fair, all of those steps are optional. Steps 1 through 3 are only done once.

But yes, to me, this is the same as unsubscribing from an issue after voting. The only difference is that using discussions, you won't get the +1 notifications before you vote. I'm ok with using discussion comments for the vote because to me it's basically the same as the status quo, but I was looking for something more in #104 .

@Andersson007
Copy link
Contributor Author

  1. Click the down arrow on the Watch/Unwatch button
  2. Choose Custom
  3. Choose at least Issues
  4. After voting in the discussion, click Unsubscribe in the discussion

Looks a bit complicated to me. Personally, I'd rather live with just ignoring +1 / -1 comments. Seems easier to me.

@mariolenz , as @briantist mentioned, steps 1-3 need to be done only once. Benefits of this compared to ignoring votes:

  1. You don't get notifications when someone votes before you vote
  2. After you vote in the discussion and click unsubscribe there, you won't get further vote notifications

I.e. after you do steps 1-3 only once, the vote algorithm is simple: vote + click unsubscribe in a discussion.

I can see now that many folks already voted in the discussion and i've got nothing, feels good:)

@Andersson007 Andersson007 changed the title [Vote ends on 2022-06-03] What to do with a lack of collection inclusion reviews? [Vote ended on 2022-06-03] What to do with a lack of collection inclusion reviews? Jun 3, 2022
@Andersson007
Copy link
Contributor Author

The vote has ended.
Results counted in the discussion:

  • Community: 2 +1, nobody is against
  • Steering Committee: 8 +1, nobody against

Could anyone from the Steering Committee please confirm my numbers?

@markuman
Copy link
Contributor

markuman commented Jun 3, 2022

I count the same numbers this time.

@Andersson007
Copy link
Contributor Author

@markuman thanks for the confirmation!

Thanks everyone! Since now we can start promoting the inclusion reviews via Bullhorn and everywhere

Repository owner moved this from Fresh to Resolved in Community Topics TODO Jun 3, 2022
@Andersson007 Andersson007 added implemented and removed active-vote These are currently active votes labels Jun 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants