Add Committer role to the project

[matks]

Questions	Answers
Description	This Pull Request is a proposal to add the role of Committer in the project. A Committer is a contributor who, after demonstrating his skills and motivation towards helping the review of contributions, would be granted a partial approval power on pull requests for the Core repository. As you can see in the PR content, a committer would be able to approve Pull Requests however a Pull Request would be accepted only if at least one maintainer approved it. So the current rule is "on the core repository, every pull request needs to be approved by two maintainers". The rule would become "on the core repository, every pull request needs to be approved by two maintainers or by one committer and one maintainer". This is really about helping, a reviewer or multiple committers cannot alone get a PR approved and mergeable. Maintainers keep the ability to decide if it’s merged or not as gatekeepers for the project. Becoming a committer could become a first step towards being a maintainer. I did not include this role on other repositories because other repositories only require one approval so the "partial approval" would not apply. This is a first step, I think committers could (and should) be granted further rights as we experiment with the concept and trust more and more community people.
Fixed ticket

This suggestion is inspired by Drupal committers and NodeJS committers.

[NeOMakinG]

I've a question

• Do github permit to have a special role for this, counting the review as a valid review in order to merge the PR?
• Can this role be scoped on different context? (front-end, back-end)

[matks]

• Do github permit to have a special role for this, counting the review as a valid review in order to merge the PR?

Yes, GitHub allows us to configure

• the number of required approvals (= 2 on the main repository)
• at least 1 approval from 1 group (= maintainer)

[NeOMakinG]

@Hlavtox @mparvazi

Can't wait to see you in that review group at least! You deserve it as your review often has a lot of value!

[matks]

@Hlavtox @mparvazi

Can't wait to see you in that review group at least! You deserve it as your review often has a lot of value!

Indeed we can add them to the list already :) if you agree

[Progi1984]

The role must be defined (context, missions, acceptance criteria) before accepting new people.

[matks]

The role must be defined (context, missions, acceptance criteria) before accepting new people.

Context and mission: already provided

Reviewers help maintainers review code contributions.

It is a subset of the context and mission of maintainers:

Maintainers provide the technical vision for the project, review code contributions, and ensure that the software is developed according to the product vision.

Criteria: this document title is "project organization".

The criteria of maintainers have not been written inside this document. This document also does not contain the criteria to apply as issue managers or any other roles. I do not think the criteria to apply should be written in there.

Maybe criteria should go into a subpage inside Maintainer Guide, as Reviewer role is a subset of Maintainer role?

[saulaski]

To keep the documentation simple and lighter to read, perhaps it should only mention "reviewers" alone instead of "maintainers and reviewers" ?
We could indeed consider all maintainers as reviewers by default, as a basic role, but they have extended roles beyond review.

[NeOMakinG]

The role must be defined (context, missions, acceptance criteria) before accepting new people.

I would also say that we should add some roles scopes like Front-end and Back-end

• Back-end stuff in this project means that you master different languages, it's not like you were a JS backend engineer, it means that you are a PHP engineer
• Front-end scope is really big now, it evolved a lot and I really think that It's hard to be "Full stack" in such a project, it means that you are a PHP, JS, and HTML/CSS engineer

[Hlavtox]

Also, if such a role is created, it should have some kind of argumentation power.

Now, when I checked one of the maintainers PR and suggested some CSS changes because it was really wrong, he just clicked "Resolved" (=I don't care).

If Valentin didn't step in and said "Yeah it's really crap", it would get merged.

[Hlavtox]

Front-end scope is really big now, it evolved a lot and I really think that It's hard to be "Full stack" in such a project, it means that you are a PHP, JS, and HTML/CSS engineer

+1 Don't take it wrong, but the quality of HTML/CSS not written by @NeOMakinG is terrible. 😄

[eternoendless]

I suggest changing the "reviewer" title to "committer"

[Progi1984]

I disagree. New roles should be added with all informations (acceptance criteria is the most important). You can modify the subpage in this PR.

[matks]

Hello everybody,

I tried to meet every feedback

The role must be defined (context, missions, acceptance criteria) before accepting new people.

I removed the new people. This PR now onlys introduces the Committer role. No candidate is mentioned.

I would also say that we should add some roles scopes like Front-end and Back-end

So you think people should apply as

• a full-stack committer
  or
• a back-end committer
  or
• a front-end committer

Also, if such a role is created, it should have some kind of argumentation power.

We can explore this but I don't think we need to grant such power. If such power is needed, it's because two people of the organization cannot talk together and agree on something. This is very bad. We try to choose candidates that are nice people and are able to talk through such disagreements 😊 . Our Code of Conduct even enforces that we listen to each other. So I don't think we need to add in the role description "what happens if you need to clash". 😅

New roles should be added with all informations (acceptance criteria is the most important). You can modify the subpage in this PR.

Upon your suggestion I can add a voting system to grant Committer role to people.

1. A maintainer states he wants to grant Committer role to developer John Doe
2. A vote is opened, it must receive a simple majority approval to grant the role. Maintainers and committers have one vote.

[matks]

🆙 😄

[kpodemski]

@matks

Upon your suggestion I can add a voting system to grant Committer role to people.
A maintainer states he wants to grant Committer role to developer John Doe
A vote is opened, it must receive a simple majority approval to grant the role. Maintainers and committers have one vote.

Simple majority. Sounds good.

So you think people should apply as

a full-stack committer
or
a back-end committer
or
a front-end committer

I'm not sure about this. Maybe it should state that we can give review permission to one or more repositories based on the reviewer's skillset. Any developer might review some parts of the documentation. Having a split between roles in this manner is overkill IMHO.

Outdated

[matks]

In order not to keep this PR waiting for longer, I can suggest I add the Vote part to this PR and then we move forward and merge this of we got approvals?

So I create a page "How to become committer" similar to https://github.com/PrestaShop/open-source/blob/master/content/maintainers-guide/how-to-become-a-maintainer.md

[kpodemski]

@matks 🆗

[matks]

Good evening everyone and apologies for the delay. I added a "How to become a committer" page very similar to "How to become a maintainer"

[matks]

Ping @kpodemski

[Progi1984]

@ttoine How and Who can add a new role in the project ?

[jolelievre]

HI @matks

ok for the content in the pages. However, is it on purpose that a binary hugo has been added in this PR?

[matks]

ok for the content in the pages. However, is it on purpose that a binary hugo has been added in this PR?

@jolelievre No 😭 I rebased the PR yesterday and I missed the line because it was so small

[jolelievre]

Ok so I block it, just in case, until the file is cleared.

[jolelievre]

Request changes don't block the merge anyway 😅. Maybe the configuration of this repository needs some love a bit ^^

[Progi1984]

Some feedbacks

[Progi1984]

Some feedbacks

[MeKeyCool]

I would also say that we should add some roles scopes like Front-end and Back-end

So you think people should apply as

* a full-stack committer
  or

* a back-end committer
  or

* a front-end committer

Then why not creating wording, product and qa scopes as well ? 🤔
Probably to early in project, this "scope" idea may be another issue i think cause it would induce lot of debates 🤔

[MeKeyCool]

Will committers allowed to assign tasks from https://github.com/PrestaShop/PrestaShop/projects ?

Outdated

[matks]

Hi everyone,

As 6 people (a majority of maintainers) have approved the PR (me, Jonathan, Matthieu, Valentin, Pablo and Krystian) I move forward and merge the PR, but I also open a GitHub issue to handle the questions of about the activity line.

[matks]

@MeKeyCool

Then why not creating wording, product and qa scopes as well ? 🤔

I think we'll have to do it 😄 . But the people of these teams will do this, not us.

Will committers allowed to assign tasks from https://github.com/PrestaShop/PrestaShop/projects ?

Even contributors can be assigned tasks, the only problem is that GitHub prevents people that are not from the organization to do it. Committers, as part of the organization, should be able to assign on a task I think.