Agree, it would bring us a step further. The only problem I see is when a code owner disappears or has no time for a longer period, that the pr get then blocked. So I think it would be not bad to have at least for every entry in the file two owners.

That's why it should be teams versus individuals, but again, lacking having teams set up in GitHub at the moment the use of individuals is there to just demonstrate how it all works.

Sounds like a no-brainer to me. Lets try it and see how it goes.

I guess if a code owner disappears, we can (and obviously should) remove him from the list and the PR gets mergeable again.

What about adding the .github folder files to the cms-maintainers group so we have a showcase for that too? ;-)

Maybe we can also add @Bakual and @infograf768 for the lanuage stuff. I mean the php classes and components.

I agree with @Bakual lets see how it goes.

If this get merged up please add me for the httpheaders plugin in 4.0 @wilsonge ?

The only problem I see here is that just people with merge access can be added to that group. I don't think it would be wise to have so many groups and people with merge access.

And by the way this would also the reason why the codemirror part don't work as he don't has merge permissions

https://help.github.com/articles/about-codeowners/

Again, this is not a finalized personnel roster, please do not scrutinize line by line who is or is not listed as an "owner" of a section; the examples used here are based on current practical work where people have been commonly engaged in sections of the code base and are either informally an "owner" of it (such as with the CodeMirror editor updates), or are a subject matter expert in the system (such as with custom fields). So while it is a somewhat accurate summary of who knows stuff or works in a certain section of code, it's not meant to be a finalized or all inclusive list.

The only problem I see here is that just people with merge access can be added to that group. I don't think it would be wise to have so many groups and people with merge access.

This is also part of the problem aiming to be addressed. Sure, it means it requires more people to have write access, but it also means that the people merging pull requests (most frequently right now myself, @wilsonge, and @laoneo; to a lesser degree you @zero-24 and @rdeutz are merging things but only matching selective criteria) don't have to be subject matter experts on the entire code base.

In a perfect world with things being the way they are right now, whenever I merged anything I would do my own final test and review of the pull request as a final validation of the change proposal (in part it acts like a checks and balances system, I have found issues on pull requests marked RTC which indicates the patch wasn't actually tested fully) and then merge it if my test and review are acceptable. The simple fact is I don't have the time for that, and I'm sure other mergers would say the same, so a lot of us are merging either with trust in something having been set RTC or after doing a quick scan over the diff to ensure there are no glaring errors.

I'm not an expert on every single feature in the CMS repository. So for me, what might come across as a simple pull request like #19982 isn't something I can just look at and be like "oh, this is right, good to go" or "umm, there's a problem here, needs work" except for obvious code syntax issues. Then there are things like #20068 which are in their fourth or fifth iteration (luckily the people involved have been willing to stay engaged with that work because it has had a lot of scrutiny and review) but even when I did hit the merge button I had to rely a lot more on the fact that others had tested and reviewed it (including the "owner" of that code).

So yes, more people have merge capabilities. Which is why the owners should be trusted individuals. If you have any familiarity with the core team structures of other projects, and I will call out WordPress, Drupal, and Symfony as three where they have something like this, it is in essence defining a "component maintainer" where someone is the owner/overseer of that section of code; in some cases these owners do have merge rights whereas in other cases what they do would count as a requirement for one of our PRs to be marked RTC.

Assuming I've accurately set up our OpenHub page as it relates to what directories it excludes from metrics, there are over 540,000 "useful" lines of code in this repository (actual production code or inline documentation/comments) on the staging branch alone, we can assume 4.0 is similar for the sake of discussion. As I pointed out earlier for all practical purposes there are 5 people actually merging patches to the active branches on this repository, and because there is no defined "trusted individual/group" charged with oversight of sections of the code, those 5 people in essence have to be absolute subject matter experts on a massive amount of code (108,000 lines each if things were split up evenly). This is about as unsustainable as only having 3 people in the entirety of the project who are trained/capable of tagging and issuing releases.

Sounds good to me let us give it a try.

(most frequently right now myself, @wilsonge, and @laoneo; to a lesser degree you @zero-24 and @rdeutz are merging things but only matching selective criteria)

Well you are the release lead and if not something changes you are in charge of taking final decision (== merge) so of course you do the most merges at the moment. Same applies to George and Allon about Joomla 4. At the time Robert was release lead he merged the most. This is absolutely what I expect from the release lead take decisions (== merges)

By the time I was given merge rights it was not intended not by me nor by the persons who gave me merge access that I'm going to merge all kind of PR's myself. I needed to have merge rights to close PRs assign labels and milestones and it was also said that i can merge basically "no-brainer PR's" that just need someone to hit the merge button. And well this is the kind of thing I do today.

If you have any familiarity with the core team structures of other projects, and I will call out WordPress, Drupal, and Symfony as three where they have something like this, it is in essence defining a "component maintainer" where someone is the owner/overseer of that section of code; in some cases these owners do have merge rights whereas in other cases what they do would count as a requirement for one of our PRs to be marked RTC.

Well I don't have that much insight in the process that they do and how many "code owners" they have and how may of them have merge rights nor how they work is structured in details to comment on that. But I fully agree on the second part that the code owner review could be a requirement to set a PR RTC.
But to me that code owner don't need to have merge permissions and that is what I would raise with my comment. I know this is a problem with how GitHub set up that feature but this is the reason why I have said this.

This is about as unsustainable as only having 3 people in the entirety of the project who are trained/capable of tagging and issuing releases.

Well in the past these three people are the only one who did releases. And to me it is the absolute power of the release lead to do the release and I would not question that. And if not assign and delegate that task. But this did not happen until now. So here we are three people having access to the update server and all the places you need to update for a release.

I don't say that all the things are great now and we don't need to change anything nor I'm denying what you said about code lines etc. I fully agree with you to get more reviews from other members of the community / code owners.
But to me this don't mean we need to extend our members with merge rights. As to me merge is decision and this is the position of the release lead.

I just don't want us to have too many people with merge rights (== decision) that could lead into situations where one person/group decide we go in this direction and another we go in this direction. To me the direction choice is primary done by the release lead. Maybe I'm alone with that opinion at all maybe not.

even without merge rights the "code owners" approval can help the "release lead" to speedup the decision, so lets try it and see how it goes.

So, to counter some of that.

I don't feel it is my responsibility to merge every pull request. I do
feel it is my responsibility to ensure those merging on my branch(es) are
staying in line with established policy and whatever vision is in place for
the release, and therefore should be able to entrust others to make
informed decisions to either merge a PR, request changes if they review and
see an issue, or ping another decision maker (someone who can merge or
release lead) for a second opinion. Generally I'd have no issue with
anyone on the owners file merging a pull request affecting their area so
long as procedure is followed (SemVer type stuff, gets to RTC by testing,
or "no brainer" things), to me that is part of the trust and communication
that should exist at this level.

Speaking of the "no brainer" stuff, PR #20135 is one of those PRs which
falls into that category, you or I might have merged it with no questions
asked, but Brian looked at it and found that although the PR does as
advertised (fixes an inconsistency) it also was the incorrect fix. So even
these simple things, having a subject matter expert doing a quick glance
before merging acts as a quality control layer.

I don't know off hand how merges are handled in WordPress and Drupal, but
the Symfony component owners do have merge rights on their sections; this
is documented in their core team guides. If I'm not mistaken, WordPress
has over 30 "components" and each one has an owner (not sure if there is
overlap, as in someone owning multiple) and I do believe many of them have
merge capability.

On Wed, Apr 11, 2018 at 12:55 PM zero-24 notifications@github.com wrote:

(most frequently right now myself, @wilsonge https://github.com/wilsonge,
and @laoneo https://github.com/laoneo; to a lesser degree you @zero-24
https://github.com/zero-24 and @rdeutz https://github.com/rdeutz are
merging things but only matching selective criteria)

Well you are the release lead and if not something changes you are in
charge of taking final decision (== merge) so of course you do the most
merges at the moment. Same applies to George and Allon about Joomla 4. At
the time Robert was release lead he merged the most. This is absolutely
what I expect from the release lead take decisions (== merges)

By the time I was given merge rights it was not intended not by me nor by
the persons who gave me merge access that I'm going to merge all kind of
PR's myself. I needed to have merge rights to close PRs assign labels and
milestones and it was also said that i can merge basically "no-brainer
PR's" that just need someone to hit the merge button. And well this is the
kind of thing I do today.

If you have any familiarity with the core team structures of other
projects, and I will call out WordPress, Drupal, and Symfony as three where
they have something like this, it is in essence defining a "component
maintainer" where someone is the owner/overseer of that section of code; in
some cases these owners do have merge rights whereas in other cases what
they do would count as a requirement for one of our PRs to be marked RTC.

Well I don't have that much insight in the process that they do and how
many "code owners" they have and how may of them have merge rights nor how
they work is structured in details to comment on that. But I fully agree on
the second part that the code owner review could be a requirement to set a
PR RTC.
But to me that code owner don't need to have merge permissions and that is
what I would raise with my comment. I know this is a problem with how
GitHub set up that feature but this is the reason why I have said this.

This is about as unsustainable as only having 3 people in the entirety of
the project who are trained/capable of tagging and issuing releases.

Well in the past these three people are the only one who did releases. And
to me it is the absolute power of the release lead to do the release and I
would not question that. And if not assign and delegate that task. But this
did not happen until now. So here we are three people having access to the
update server and all the places you need to update for a release.

I don't say that all the things are great now and we don't need to change
anything nor I'm denying what you said about code lines etc. I fully agree
with you to get more reviews from other members of the community / code
owners.
But to me this don't mean we need to extend our members with merge rights.
As to me merge is decision and this is the position of the release lead.

I just don't want us to have too many people with merge rights (==
decision) that could lead into situations where one person/group decide we
go in this direction and another we go in this direction. To me the
direction choice is primary done by the release lead. Maybe I'm alone with
that opinion at all maybe not.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#20137 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoYngWIbmcoPohRadnCzgdA08YUnZks5tnkOlgaJpZM4TPYBE
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

I will post this idea that I've shared some time ago:
Lets break Joomla to 4 major parts:

• HTML: Accessibility team is essentially controlling of the related code.
• CSS: The design (?) team is controlling all aspects of the related code.
• JS: The JS group govern this part of the code.
• PHP: Production leader, release leader, maintenance and security rule this code.

So basically all I am trying to say here is: we already have a foundation on who's responsible for what by the languages themselves. We just need an official design team (closely collaborating with A11Y and JS because the delivered pages do have all these 3 languages).

Now the idea to fragment the code to even smaller pieces where one person will be in charge of some smaller or bigger part, although is great, I think it will be very hard to execute (find so many contributors willing to take such a responsibility, but I might be wrong here).
Anyways that was my 2c here

Now the idea to fragment the code to even smaller pieces where one person will be in charge of some smaller or bigger part, although is great, I think it will be very hard to execute (find so many contributors willing to take such a responsibility, but I might be wrong here).

i don't read this proposal like this:

i've read it something more like this:
fragment the code to smaller pieces where a group of people or someone in that specific group can turn on a green light to let the "release lead" or whatever you call it to have at least a

quality control layer.

it will be always the "release lead" responsabilty to merge it or to take the final decision

Having those 4 core groups is a good starting point, but the PHP API is too
large and complex to depend on either people being randomly pinged, paying
attention to every PR, or expecting a small group to "own" it all.

Take fields as an example; it's Allon's code. I'm not familiar with it's
workflow or structure or integrations or design decisions, so it's
difficult for me to be involved with review on it. But he can. Let him
manage that and me focus elsewhere.

Take our APIs with database integration (extension install library, schema
manager as two high level examples). Most people can probably deal with
the root concept of it no problem, especially as testing things on MySQL is
generally a breeze. But changes aren't always validated against PostgreSQL
or SQL Server. I would argue some aspects of database integration related
PRs need to be "blessed" by a person capable of judging a database's
capabilities and integrations.

It's why I think breaking things down a bit distributes the workload off of
some overworked/overtaxed individuals. How that works specifically still
needs some hashing out, but the status quo isn't sustainable in various
ways and we need to optimize that a bit.

On Wed, Apr 11, 2018 at 1:44 PM Dimitri Grammatikogianni <
notifications@github.com> wrote:

I will post this idea that I've shared some time ago:
Lets break Joomla to 4 major parts:

• HTML: Accessibility team is essentially controlling of the related
  code.
• CSS: The design (?) team is controlling all aspects of the related
  code.
• JS: The JS group govern this part of the code.
• PHP: Production leader, release leader, maintenance and security
  rule this code.

So basically all I am trying to say here is: we already have a foundation
on who's responsible for what by the languages themselves. We just need an
official design team (closely collaborating with A11Y and JS because the
delivered pages do have all these 3 languages).

Now the idea to fragment the code to even smaller pieces where one person
will be in charge of some smaller or bigger part, although is great, I
think it will be very hard to execute (find so many contributors willing to
take such a responsibility, but I might be wrong here).
Anyways that was my 2c here

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#20137 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfofA68XBoTkryactugFMrA2u1WXp-ks5tnk7zgaJpZM4TPYBE
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

@zero-24, we are talking here about reviews. I mean even when people get merge rights, the primary job of a code owner is to approve a pr.

I agree with this totally. Who wants to own setting up this file?

Umm this is a pull request every thing you need to do is here

yah i was even filtering by pull requests when i opened this issue. i definitely need to go sleep.....