zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.5k stars 6.43k forks source link

Lack of rules to avoid intentional abuse of power in someone else's subproject #77255

Closed avolkov-1221 closed 6 days ago

avolkov-1221 commented 3 weeks ago

Introduction

This issue was raised in discussions and during the sequence of actions for PR #75740. The current Zephyr policy and documentation do not clearly explain the roles and permissions of "collaborators" and "maintainers" outside of their subprojects.

Problem description

The "collaborator" user from one project has added the 3rd person from his company as a "maintainer" to the unrelated project and took the maintainer's role for this project without any public announcement or permission.

Proposed change

Roles and permissions have to be described in more detail. In particular, what exactly users with write permissions are allowed to do outside of there subprojects. Also rules for revoking permissions from violators should be added.

Detailed RFC

Proposed change (Detailed)

Clarification will help reduce potential conflicts of interest and avoid the nasty "Embrace, Extend and Extinguish" practice, where someone with write permission is hired by a company and allows that company to shortcut or, worse, to bypass the commit acceptance process without testing, discussion, or taking the community's interests into account.

Dependencies

Concerns and Unresolved Questions

Alternatives

jfischer-no commented 3 weeks ago

@avolkov-1221 Is it about https://github.com/zephyrproject-rtos/zephyr/pull/75740? The bot assigned it incorrectly, probably due to clock control changes. But I guess it is not just that, right?

avolkov-1221 commented 3 weeks ago

Yes,

@avolkov-1221 Is it about #75740? The bot assigned it incorrectly, probably due to clock control changes. But I guess it is not just that, right?

Yes, it's. The problem is not in the bot, but the overstepped authority by Yiannis Damigos ("collaborator"): he has made a person with no experience with Zephyr as a "maintainer", and then assigned him to a competing PR as an "Assignees".

avolkov-1221 commented 3 weeks ago

@avolkov-1221 Is it about #75740? The bot assigned it incorrectly, probably due to clock control changes. But I guess it is not just that, right?

PS @jfischer-no my last reply is here https://github.com/zephyrproject-rtos/zephyr/pull/75740#issuecomment-2299185060

jfischer-no commented 3 weeks ago

https://github.com/zephyrproject-rtos/zephyr/pull/75498 updated Renesas RA Platforms and hal_renesas maintainers. I would not blame Yiannis Damigos. The PR was approved and merged as usual. The trust probably comes from the fact that this vendor joined the project. Whether a vendor then inherits all the boards, and a vendor representative becomes a maintainer, is an interesting question. By the way, it is not that dramatic; the person with no experience is not the only maintainer in the area.

avolkov-1221 commented 3 weeks ago

75498 updated Renesas RA Platforms and hal_renesas maintainers. I would not blame Yiannis Damigos. The PR was approved and merged as usual. The trust probably comes from the fact that this vendor joined the project. Whether a vendor then inherits all the boards, and a vendor representative becomes a maintainer, is an interesting question. By the way, it is not that dramatic; the person with no experience is not the only maintainer in the area.

I agree that Yannis probably acted with the best of intentions, and I don't really wish to blame him either, but the result doesn't look so good in any case. We have a real 'conflict of interest' with Renesas (they wish to push their FSP's dependency at any cost), which I personally resisted. As a result, my PR is frozen, regardless of the quality of my code, but due to personal decision. And we don’t have a clear arbitration on how to resolve this situation. IMO "assignee" have to be neutral 3rd party person, not from competitive subproject.

I think that the situation with 2 competing PRs at the same time will most likely not happen again, however the situation with overstepped authority is likely to happen repeatedly. Therefore, I propose to clarify the hierarchy and boundaries of authority, in order to avoid such conflicts in the future. For example, either the project maintainer or the top-level one, as an exception, can add a new maintainer/collaborators and reassign "assignee", but not some person from unrelated subproject.

As far as I remember, this is exactly how it's done in Linux: maintainers outside of their projects are not so different from regular developers and do not have increased permissions. I propose to reuse it, only replacing almighty Linus with WG :).

ydamigos commented 3 weeks ago

@avolkov-1221 Is it about #75740? The bot assigned it incorrectly, probably due to clock control changes. But I guess it is not just that, right?

PS @jfischer-no my last reply is here #75740 (comment)

@ydamigos Are you kidding me? It was you, who pushed him through as a maintainer, violating Zephyr's rules:

He became a maintainer without a single verified commit, just based on trust in you.

PR #75498 was approved by the maintainer of RA platform before it was merged. Moreover, I asked specifically about it https://github.com/zephyrproject-rtos/zephyr/pull/75498#issuecomment-2217892555.

And the first PR from this team contained serious violations of many of Zephyr's code guide requirements, which shows that they lack experience. He shouldn't have become a "maintainer", nor should they have become "collaborators" without proper preparation.

Are we talking about PR #72150. Please provide a list of the violations so we can address them. Afaik, Zephyr's code guide requirements don't apply to vendor HAL, do they? Kheim, Thao and Duy are very familiar with RA family and FSP. I believe they are perfect candidates for maintainer and collaborators role. They may lack experience with Zephyr code base but we should support them and help them to gain it. I believe exposing them to Zephyr's community is the perfect and proper preparation for them to grow as developers (I am talking from personal experience). As I see it, it's a win win situation for Zephyr project / community and them.

Then, as a "collaborator", not "maintainer", in an external project, you overstepped your authority and assigned him responsible for a competing PR.

I just added one of the maintainers, should I add or change it to @soburi instead?

Yannis, this looks like some dirty political game on your part to pushing vendor lock at any cost, rather than proper code collaboration within the community.

There was already PR #71174 which added support for RA2 family which using FSP not originating from Renesas which was approved and merged. I don't believe that Renesas team is doing anything "behind the scenes".

fabiobaltieri commented 3 weeks ago

@avolkov-1221 it's a bit tricky to understand the issue if you don't reference to the controversial PR/issues, at least we have inspector @jfischer-no to the rescue, but seriously, just link the PR/issues that you think need to be reevaluated an then one could check them out and draw some conclusion, I really don't see the point in being vague or trying to hide the fact, people are just going to either waste time searching for the data or disengage. Anyway, I see two problems here:

We have a real 'conflict of interest' with Renesas (they wish to push their FSP's dependency at any cost), which I personally resisted. As a result, my PR is frozen, regardless of the quality of my code, but due to personal decision. And we don’t have a clear arbitration on how to resolve this situation. IMO "assignee" have to be neutral 3rd party person, not from competitive subproject.

This sounds like an architecture issue, should probably go through the Arch WG, ideally with a presentation of the pro/con of each approach (I'm assuing FSP is a Renesas hal and the discussion is whether to use it or not?). This is a technical problem and should be threated as such, the project should evaluate and adopt the technically superior solution.

Yes, it's. The problem is not in the bot, but the overstepped authority by Yiannis Damigos ("collaborator"): he has made a person with no experience with Zephyr as a "maintainer", and then assigned him to a competing PR as an "Assignees".

This is more of a process problem, it may make sense to have stricter criteria for the various roles, maybe have some automation to further restrict who can be set as "assignee" to a pull request since that field has lot of implication (this came out at release meeting yesterday, again with no context, but it makes more sense now).

nashif commented 3 weeks ago

The "collaborator" user from one project has added the 3rd person from his company as a "maintainer" to the unrelated project and took the maintainer's role for this project without any public announcement or permission.

if you are referring to https://github.com/zephyrproject-rtos/zephyr/pull/75498:

from his company as a "maintainer" to the unrelated project

Anyone can propose collaborators to other areas of the project, there is no limitation on this. This will still have to be reviewed and approved.

Unless I am missing something else, I do not see any violation or abuse of power here.

avolkov-1221 commented 3 weeks ago

@fabiobaltieri

@avolkov-1221 it's a bit tricky to understand the issue if you don't reference to the controversial PR/issues, at least we have inspector @jfischer-no to the rescue, but seriously, just link the PR/issues that you think need to be reevaluated an then one could check them out and draw some conclusion, I really don't see the point in being vague or trying to hide the fact, people are just going to either waste time searching for the data or disengage.

I've tried to elevate and concentrate here only on the hierarchy/permissions/borders of authority problem, without involving the personalities. Sorry if not so successful. I've added the PR id to the description.

Anyway, I see two problems here:

We have a real 'conflict of interest' with Renesas (they wish to push their FSP's dependency at any cost), which I personally resisted. As a result, my PR is frozen, regardless of the quality of my code, but due to personal decision. And we don’t have a clear arbitration on how to resolve this situation. IMO "assignee" have to be neutral 3rd party person, not from competitive subproject.

This sounds like an architecture issue, should probably go through the Arch WG, ideally with a presentation of the pro/con of each approach (I'm assuing FSP is a Renesas hal and the discussion is whether to use it or not?). This is a technical problem and should be threated as such, the project should evaluate and adopt the technically superior solution.

Agree, the "FSP" (and more generally, manufacturers HALs in the modules) is other problem, and IMHO now is a real headache. Actually almost unrelated to the topic. Would you like me to create a separate ticket for this?

Yes, it's. The problem is not in the bot, but the overstepped authority by Yiannis Damigos ("collaborator"): he has made a person with no experience with Zephyr as a "maintainer", and then assigned him to a competing PR as an "Assignees".

This is more of a process problem, it may make sense to have stricter criteria for the various roles, maybe have some automation to further restrict who can be set as "assignee" to a pull request since that field has lot of implication (this came out at release meeting yesterday, again with no context, but it makes more sense now).

Exactly. However, I suppose that for now, simply updating the documentation should be enough: reassigning the "asignee" already requires elevated status, some experience, and involvement in Zephyr's development. For a random external person do it not so easy.

fabiobaltieri commented 3 weeks ago

I've tried to elevate and concentrate here only on the hierarchy/permissions/borders of authority problem, without involving the personalities. Sorry if not so successful. I've added the PR id to the description.

Not a problem, I acknowledge and admire the effort, but I think that the context is important and leaving it out for the sake of not pointing finger is just confusing.

avolkov-1221 commented 3 weeks ago

@nashif

The "collaborator" user from one project has added the 3rd person from his company as a "maintainer" to the unrelated project and took the maintainer's role for this project without any public announcement or permission.

if you are referring to #75498:

  • This is the documented process.
  • change was submitted and was reviewed and approved, this is the same as "public announcement" (submitting a PR) and permission (Review + Approval)

from his company as a "maintainer" to the unrelated project

Anyone can propose collaborators to other areas of the project, there is no limitation on this. This will still have to be reviewed and approved.

The proposal of colleagues and acceptance them for any roles is entirely legal, but the issue is somewhat different: persons without any real experience in Zephyr development were proposed and accepted for roles with extended permissions, whereas Zephyr's "meritocracy" policy does not suppose it. At least, there qualification should be confirmed before, through accepted PR(s).

Unless I am missing something else, I do not see any violation or abuse of power here.

The "abuse of power" in this case occurred when a developer with "collaborator" role in one subproject started acting as a maintainer in a subproject that was not directly related to him. As I understand it, he had to do this exactly because the colleagues he recommended did not have sufficient experience. Moreover, he assigned this colleague as the "assignee" for a competing PR that already had unresolved "conflict of interests", which only made the situation worse. It started to look like unfair competition.

fabiobaltieri commented 3 weeks ago

Agree, the "FSP" (and more generally, manufacturers HALs in the modules) is other problem, and IMHO now is a real headache. Actually almost unrelated to the topic. Would you like me to create a separate ticket for this?

Yeah, based on what I see here it seems like it may be worth keeping it as a separate conversation.

keith-zephyr commented 3 weeks ago

Process WG 2024-08-21:

@nashif - The maintainer nomination for the HAL and platform followed existing processes. @nashif - Generally contributors from confirmed vendors are implicitly trusted. @KhiemNguyenT - background for this issue. Renesas became a Zephyr member at the beginning of the year. Initial support was for one MCU device. Renesas has internal development that is not upstream. Generally the process has worked, but one PR has been in conflict. @nashif - doesn't agree that there was an abuse of power. @jfischer-no - it would be preferred to add new members at contributors prior to elevating to maintainer role. @fabiobaltieri - Issue was a technical decision overridden by a new member. @dleach02 - the process was followed. @fabiobaltieri - is it okay to add folks to maintainer list without support artifacts? @nashif - in this case yes because the maintainer came from the vendor @dleach02 - the processes are okay @nashif on https://github.com/zephyrproject-rtos/zephyr/pull/75498/files 2 nominations has supporting material, 3rd was a vendor rep. The PR went through all necessary reviews. @KhiemNguyenT - history shows Renesas submitted a patch first before @avolkov-1221. @dleach02 - maintainers should try to resolve PR conflicts, but it can be escalated if needed. Who submits a PR first isn't the only consideration. @kartben - it appeared that the wrong person was assigned to the PR in conflict. A collaborator changed the assignee. @nashif - maintainers need to be taking a more active role. There was no ill intention. @kartben - What are the rules for changing assignee? How do new users discover this? @dleach02 - do we have clear rules on how the assignee is selected? This is buried in the script and not documented. @jfischer-no - Agrees the PR reassignment was the correct action. Thinks collaborators should have the right to reassign PRs. One issue was the maintainer was added in one step to the platform area. @nashif - It's common for new soc/vendor support to add maintainers that have not been in the project before. Vendors have the expertise for the hardware they are adding. @fabiobaltieri - this issue was for an existing platform, and the maintainer list changed while the PR was in flight. @nashif - agrees we could use some clarification, but doesn't agree the specific PR in conflict was an abuse of power. @nashif - with new subsystems, it's unavoidable that we'll have new contributors elevated to maintainer. @dleach02 - maintainers have the option to delegate assignee to a collaborator. @nashif doesn't agree - assignees should be a maintainer. @jfischer-no - currently does this delegation, but keeps watch over these PRs. @nashif - there are no checks to prevent non-collaborators/maintainers from getting assigned a PR. @nashif - the project does need better guidance on how to resolve technical disputes. @dleach02 - the escalation process defines this. @dleach02 - found documentation that explicitly indicates assignees should be a maintainer. No language about how to delegate transition. @keith-zephyr - what are the next steps for @avolkov-1221? @dleach02 - the PR in dispute needs to follow the established escalation process. @dleach02 - is the Process WG okay with expanding the assignee rules to allow maintainers to delegate to collaborators? @nashif - suggest the maintainer should expand the maintainer pool instead and fit in the existing process. @nashif - generally changes to platform area actually impacts the greater project, so the assignee needs to be from the affected subsystem and no the platform.

dleach02 commented 3 weeks ago

I agree that Yannis probably acted with the best of intentions, and I don't really wish to blame him either, but the result doesn't look so good in any case. We have a real 'conflict of interest' with Renesas (they wish to push their FSP's dependency at any cost), which I personally resisted. As a result, my PR is frozen, regardless of the quality of my code, but due to personal decision. And we don’t have a clear arbitration on how to resolve this situation. IMO "assignee" have to be neutral 3rd party person, not from competitive subproject.

I think that the situation with 2 competing PRs at the same time will most likely not happen again, however the situation with overstepped authority is likely to happen repeatedly. Therefore, I propose to clarify the hierarchy and boundaries of authority, in order to avoid such conflicts in the future. For example, either the project maintainer or the top-level one, as an exception, can add a new maintainer/collaborators and reassign "assignee", but not some person from unrelated subproject.

As far as I remember, this is exactly how it's done in Linux: maintainers outside of their projects are not so different from regular developers and do not have increased permissions. I propose to reuse it, only replacing almighty Linus with WG :).

At the risk of treading on sensitive ground here I want to point out a couple of things:

https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#pr-review-escalation https://docs.zephyrproject.org/latest/project/project_roles.html#assignee

nashif commented 3 weeks ago
  • have the right to put a block on any PR and then defend that block

Let's avoid the language of "block" please. In github you are able to request changes that can be considered or also dismissed. When you request changes, you do not block anything, you basically providing feedback and requesting it to be considered.

avolkov-1221 commented 3 weeks ago

I agree that Yannis probably acted with the best of intentions, and I don't really wish to blame him either, but the result doesn't look so good in any case. We have a real 'conflict of interest' with Renesas (they wish to push their FSP's dependency at any cost), which I personally resisted. As a result, my PR is frozen, regardless of the quality of my code, but due to personal decision. And we don’t have a clear arbitration on how to resolve this situation. IMO "assignee" have to be neutral 3rd party person, not from competitive subproject. I think that the situation with 2 competing PRs at the same time will most likely not happen again, however the situation with overstepped authority is likely to happen repeatedly. Therefore, I propose to clarify the hierarchy and boundaries of authority, in order to avoid such conflicts in the future. For example, either the project maintainer or the top-level one, as an exception, can add a new maintainer/collaborators and reassign "assignee", but not some person from unrelated subproject. As far as I remember, this is exactly how it's done in Linux: maintainers outside of their projects are not so different from regular developers and do not have increased permissions. I propose to reuse it, only replacing almighty Linus with WG :).

At the risk of treading on sensitive ground here I want to point out a couple of things:

  • in the end, it doesn't matter whether the person was a maintainer or a collaborator or a vendor contributor. They are a contributor in this community and as such have the right to put a block on any PR and then defend that block. It is the obligations of the parties involved to discuss and attempt to come to some consensus.

You are describing a normal situation, as it should be. Before this case, I participated in development in exactly this way. However here we have a little bit different situation: maintainer/assignee actively moving forward his competing solution at the same time with reviewing mine. For me it's "conflict of interest", that should be avoided.

  • We have had instances of competing PRs multiple times and most of the time they get resolved between the contributors without having to be raised any higher. In the many years I've been with the project I actually only remember one such competing PRs issue reach all the way to the TSC level for resolution. It was unfortunate for all parties involved that it couldn't be resolved in an earlier stage

Unfortunately, until last week my requests to discuss and re-synchronize our PRs were ignored on Renesas side.

  • We have a defined process for dealing with this and escalating things. This includes the roles and responsibilities for both the contributor, any objectors, and the Assignee. Please consult the following:

https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#pr-review-escalation https://docs.zephyrproject.org/latest/project/project_roles.html#assignee

These docos don't address the situation where the "assignee" is not interested in accepting the PR due to a conflict of interest.

@nashif @jfischer-no Btw the description of escalation procedure is slightly irrelevant: I, as the author of the PR, don't have permission to change my PR's labels and can't assign the "Architecture Review" one.

ydamigos commented 3 weeks ago

Unfortunately, until last week my requests to discuss and re-synchronize our PRs were ignored on Renesas side.

@avolkov-1221 Could you help me understand what do you mean by "were ignored on Renesas side"? We were discussing on discord (https://discord.com/channels/720317445772017664/1263700276943126629), weren't we?

avolkov-1221 commented 3 weeks ago

Unfortunately, until last week my requests to discuss and re-synchronize our PRs were ignored on Renesas side.

@avolkov-1221 Could you help me understand what do you mean by "were ignored on Renesas side"? We were discussing on discord (https://discord.com/channels/720317445772017664/1263700276943126629), weren't we?

My sync request is dated July 19th. Some meaningful discussion started after you joined the thread (but again, it is not your project). However I didn't get anything reasonable that "use our HAL. Wait for the HAL, FSP, whatsoever completed", and in the same time Renesas actively pushing there competitive code.

Sorry, but we're back to the "prove qualification before assigning to any significant role, without any exceptions" topic. I think that the assumption per se about the high qualifications of the vendor's software engineers are not always correct. I suppose that almost every experienced developer here can recall similar examples.

dleach02 commented 3 weeks ago

docs.zephyrproject.org/latest/contribute/contributor_expectations.html#pr-review-escalation docs.zephyrproject.org/latest/project/project_roles.html#assignee

These docos don't address the situation where the "assignee" is not interested in accepting the PR due to a conflict of interest.

@nashif @jfischer-no Btw the description of escalation procedure is slightly irrelevant: I, as the author of the PR, don't have permission to change my PR's labels and can't assign the "Architecture Review" one.

@avolkov-1221 the process is available to all:

I'll admit that if you are not in the contributors general list I think you may not be able to put a label on it but you can post a comment in the PR requesting this label and it will be done by the folks already engaged. This is not gated by the assignee. If you disagree that the process is not clear then please suggest some clarifying language.

I recommend that you read through the links and then follow the process here. If any of the parties involved feel they are at an impasse discussing things in the actual PR or over discord then setup an agenda item in the Zephyr Dev Meeting. It is a weekly meeting on Thursday.

If that doesn't converge:

There is the weekly architecture call for more detailed discussions. And if that fails to resolve the issues the final path is to bring this to the TSC level.

kartben commented 3 weeks ago

Hey @avolkov-1221 - I would like to remind you that the Zephyr Project has a Code of Conduct, and that all our community members are expected to follow it. Making claims on someone's English skills is absolutely not OK (even more so as Khiem actually joined the Process WG meeting yesterday and there was absolutely no issue on that front, quite the opposite actually), and I am not sure why you seem to think being fluent in the English language would be a requirement to be a maintainer. I can assure you that the project would be in trouble if that would be the case :)

I understand your frustration, but please do try to keep your comments professional.

nashif commented 3 weeks ago

My sync request is dated July 19th. Some meaningful discussion started after you joined the thread (but again, it is not your project). However I didn't get anything reasonable that "use our HAL. Wait for the HAL, FSP, whatsoever completed", and in the same time Renesas actively pushing there competitive code.

Btw Yannis, it has been your choice to draw everyone's attention to this Discord's thread. I wouldn't do that. But since we've started, I'll take the next step, although I hate discussing anyone’s skills, but it's related to this topic.

@nashif the chat with @khiemng (I don't have any feedback from any of his team members), and especially last night's one clearly demonstrated that:

  • his English level is not enough for the maintainer role;
  • his knowledge of Renesas SOCs is not so well (RA2A1 and RA2LA1 related topic);
  • his knowledge of ARM arch should be improved, at least. Topic about VTOR. It’s him, as the maintainer and Renesas employee, who should explain to me what VTOR is, not vice versa.

@avolkov-1221 Sorry, but this needs to stop. What you are saying here is a violation of the project code of conduct and it is getting way too personal.

Sorry, but we're back to the "prove qualification before assigning to any significant role, without any exceptions" topic. I think the above is a good demonstration that the assumption per se about the high qualifications of the vendor's software engineers are not always correct. I suppose that almost every experienced developer here can recall similar examples.

There is nothing to prove here. The process was followed and the decision to add @KhiemNguyenT into the project was mine and based on merits and the fact that @KhiemNguyenT representing the vendor the platforms of the area in question. This is something we did in the past for other platforms.

Some aspects of the process are not perfect and can always be improved, but it should not get to such strong negative language. So let's focus on improving the process.

Re-assigning from one maintainer (wrongly assigned by bot) to another maintainer (correct maintainer based on the MAINTAINER.yml file) by a collaborator in the project maybe be not completely OK, but it was not an abuse of power as you describe, someone was trying to set the record straight because nobody else did that.

Please deal with the technical and code related issues as a converation in the PR like we do with any PR, who is assigned and if someone has the experience will be reflected in such technical discussion.

avolkov-1221 commented 3 weeks ago

I apologize to everyone (especially @KhiemNguyenT and @ydamigos), I agree that I crossed the line. I'll trying this discussion as neutral as it possible, without any personalities.

avolkov-1221 commented 3 weeks ago

Please deal with the technical and code related issues as a converation in the PR like we do with any PR, who is assigned and if someone has the experience will be reflected in such technical discussion.

@nashif let me clarify my opinion:

I don't offer to revert any already accepted decisions. I've suggested only to update the assignment guideline accordingly.

fabiobaltieri commented 3 weeks ago

I've suggested only to update the assignment guideline accordingly.

How would you suggest to update those, precisely?

avolkov-1221 commented 3 weeks ago

I've suggested only to update the assignment guideline accordingly.

How would you suggest to update those, precisely?

Something like this:

" Maintainer ..... Contributors or Collaborators are promoted to the Maintainer role by adding the GitHub user name to one or more maintainers sections of the MAINTAINERS File in the Zephyr repository.

All applicants, other than Contributors with Triage permission or Collaborators, shall prove their qualification by having an approved PR and demonstrating sufficient knowledge of English to fulfill their duties as maintainers. However any Contributor with Triage permission or Collaborator can be promoted without any additional checks. " The English language requirement is optional.

jfischer-no commented 3 weeks ago

I do not think sufficient English is a requirement (what English, by the way?). In my opinion, someone does not need to speak English at all as long as they can help themselves with tools. I do not think the discussion about this is wise or useful, but I do not speak good English either.

What I think makes sense is that a person who is not a contributor in any area of the Zephyr tree would need TSC approval to become a maintainer.

avolkov-1221 commented 3 weeks ago

I do not think sufficient English is a requirement (what English, by the way?). In my opinion, someone does not need to speak English at all as long as they can help themselves with tools. I do not think the discussion about this is wise or useful, but I do not speak good English either.

What I think makes sense is that a person who is not a contributor in any area of the Zephyr tree would need TSC approval to become a maintainer.

As I mentioned earlier, the English language requirement might be excessive and could be removed. However, some minimal qualification checks should still be performed. The maintainer has too many privileges, so he should not be a random newbie. Actually I suggested an approved PR requirement not only as a qualification test but also for educational purposes. Certain aspects that are obvious to a permanent member of the Zephyr community may be unfamiliar and unclear to a 3rd party developer, regardless of his experience and qualifications. But as an exception for some well known person, TSC approval would be probably enough.

marc-hb commented 3 weeks ago

Collaboration requires communication and communication requires a common language. I don't think pretending otherwise is "wise or useful". I have worked for a couple decades with people from various countries, various languages and varied English levels. Even when everyone does their best, the less natural language people have in common and the more difficult the communication is and the more frequent the misunderstandings are. Interestingly enough the current discussion is to some extend a demonstration: I have not seen anyone requesting "fluent" or even "good" English. At the risk of stating the obvious, a language is not just a collection of words, it also represents a culture, some social norms and it shapes the way you think. BTW how could anyone follow a Code of Conduct without a minimal understanding of it? You can't be respectful and welcoming with a poor choice of poorly translated words.

Natural language differences are hard enough between teammates in the same company, under the same direction and working towards the same goals. But maintainership and the corresponding code reviews require handling more conflicts which raises potential communication issues to a whole new level. Working over the Internet does not help either. A great maintainer has good communication skills and language is an obvious part of that.

To be clear: I have close to zero knowledge of the particular situation that triggered all this. I'm only commenting about maintainership and natural languages in general. I believe this is on-topic.

I don't have any simple solution to offer either. I'm not suggesting any language "exam" or anything like it. I realize selecting a maintainer is really hard, that volunteers are scarce, that language is far from the only factor, etc. I'm merely stating we can't pretend that English does not matter, that's simply not true. Trying to dismiss the natural language question will not make it go away. If considering the English level of maintainers is inappropriate then maybe it can be wrapped under some more general umbrella like "a record of appropriate communication skills"? Hopefully everyone can agree those skills matter.

EDIT:

(what English, by the way?)

https://en.wikipedia.org/wiki/International_English :-)

... as in: https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good

avolkov-1221 commented 3 weeks ago

Collaboration requires communication and communication requires a common language. I don't think pretending otherwise is "wise or useful". I have worked for a couple decades with people from various countries, various languages and varied English levels. Even when everyone does their best, the less natural language people have in common and the more difficult the communication is and the more frequent the misunderstandings are. Interestingly enough the current discussion is to some extend a demonstration: I have not seen anyone requesting "fluent" or even "good" English. At the risk of stating the obvious, a language is not just a collection of words, it also represents a culture, some social norms and it shapes the way you think. BTW how could anyone follow a Code of Conduct without a minimal understanding of it? You can't be respectful and welcoming with a poor choice of poorly translated words.

Natural language differences are hard enough between teammates in the same company, under the same direction and working towards the same goals. But maintainership and the corresponding code reviews require handling more conflicts which raises potential communication issues to a whole new level. Working over the Internet does not help either. A great maintainer has good communication skills and language is an obvious part of that.

To be clear: I have close to zero knowledge of the particular situation that triggered all this. I'm only commenting about maintainership and natural languages in general. I believe this is on-topic.

I don't have any simple solution to offer either. I'm not suggesting any language "exam" or anything like it. I realize selecting a maintainer is really hard, that volunteers are scarce, that language is far from the only factor, etc. I'm merely stating we can't pretend that English does not matter, that's simply not true. Trying to dismiss the natural language question will not make it go away. If considering the English level of maintainers is inappropriate then maybe it can be wrapped under some more general umbrella like "a record of appropriate communication skills"? Hopefully everyone can agree those skills matter.

Thank you so much for your addition. This is exactly what I meant: a maintainer must be able to explain his suggestions in a sufficiently clear and kind manner. If, for example, he uses the imperative mood simply due to lack of experience, it will not matter to the contributors. The result will be equally bad, regardless of his initial intentions.

So, the addendum might look like this:

All applicants, other than Contributors with Triage permission or Collaborators, shall prove their qualification by having an approved PR and demonstrate appropriate communication skills to fulfill their duties as maintainers. However any Contributor with Triage permission or Collaborator can be promoted without any additional checks. Additionally, in exceptional cases, the TSC may also promote a well known person to the position of maintainer.

EDIT:

(what English, by the way?)

https://en.wikipedia.org/wiki/International_English :-)

... as in: https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good

:))

jfischer-no commented 3 weeks ago

Collaboration requires communication and communication requires a common language. I don't think pretending otherwise is "wise or useful". I have worked for a couple decades with people from various countries, various languages and varied English levels. Even when everyone does their best, the less natural language people have in common and the more difficult the communication is and the more frequent the misunderstandings are.

Most of the time it helps to follow the context and use simpler language. Here I will stick to the point that discussing it is not wise and not helpful. Especially because the statements in this regard were not true.

032flkdj033jlf34

https://github.com/zephyrproject-rtos/zephyr/issues/77255#issuecomment-2305092258

Interestingly enough the current discussion is to some extend a demonstration: I have not seen anyone requesting "fluent" or even "good" English. At the risk of stating the obvious, a language is not just a collection of words, it also represents a culture, some social norms and it shapes the way you think. BTW how could anyone follow a Code of Conduct without a minimal understanding of it? You can't be respectful and welcoming with a poor choice of poorly translated words.

Can the barbarians not be respectful because they do not speak the languages of the noble lords?

keith-zephyr commented 2 weeks ago

The current Maintainer section in the project roles states:

Contributors or Collaborators are promoted to the Maintainer role by adding the GitHub user name to one or more maintainers sections of the MAINTAINERS File in the Zephyr repository.

To become a Contributor with Triage permission states:

Contributors who show dedication and skill are granted the Triage permission level to the Zephyr GitHub repository.

So the missing piece seems to be defining the process for adding a maintainer that is not currently a contributor with triage or a collaborator. As @jfischer-no suggested, I like the idea of requiring TSC approval for adding a maintainer that skips the contributor/collaborator steps.

I don't think adding a vague requirement of "demonstrating appropriate communication skills" is warranted.

henrikbrixandersen commented 2 weeks ago

So the missing piece seems to be defining the process for adding a maintainer that is not currently a contributor with triage or a collaborator. As @jfischer-no suggested, I like the idea of requiring TSC approval for adding a maintainer that skips the contributor/collaborator steps.

I like that idea as well.

jfischer-no commented 1 week ago

So the missing piece seems to be defining the process for adding a maintainer that is not currently a contributor with triage or a collaborator. As @jfischer-no suggested, I like the idea of requiring TSC approval for adding a maintainer that skips the contributor/collaborator steps.

I opened https://github.com/zephyrproject-rtos/zephyr/pull/77933.

keith-zephyr commented 6 days ago

Closing as fixed by https://github.com/zephyrproject-rtos/zephyr/pull/77933