Re: [webkit-dev] Sunsetting committership and reviewership
Interesting. What privileges, if any, would you propose 'emeritus reviewers' to have? -Filip On Apr 9, 2013, at 2:54 PM, Dmitry Titov dim...@chromium.org wrote: How about creating an 'emeritus reviewer' status (no r+ power) and let people *voluntarily* move themselves to this status? I bet a lot of 'inactive reviewers' would do that, since everybody understands the issue of getting out of sync with current code base. It may have different vibe though than figuring some automatic time-based enforcement system... As an added bonus, this gives such people a good way to avoid being asked to review a patch for a colleague while keeping some ties with the project... On Mon, Apr 8, 2013 at 3:34 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. We sometimes get low-quality drive-by reviews even from people who are active at the time. I feel like that's not the right basis for the time cutoff. If we do have a sunset period, we should think about it in terms of how long it takes to be so out of touch with the current state of the project that there's little chance you can give a useful review. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
Unrelated to Dmitry's suggestion, but since I brought up emeritus contributors earlier in the thread, I should explain my usage. The emeritus class proposed in the ancient webkit-reviewers thread about sunsetting was simply to answer the fact that committers.py has two purposes: 1. It exists as a public Access Control List (ACL) for svn.webkit.org (since I know of no other public ACL). 2. It exists as a way for our tools to lookup Contributor objects based on name, irc nick, svn email etc. emeritus contributors (in my original proposal on webkit-reviewers years ago) only exist to serve the second purpose, not the first. This would be similar to the Contributor and Account superclasses which have (since that old thread) been added to committers.py to allow committers.py to list non-commiters and even bots which we might want to have in our CC list, but not give any privileges to. Again, my thoughts on this may bear no reflection to what Dmitry had in mind with his use of the word emeritus, so I should let him speak! On Tue, Apr 9, 2013 at 11:39 PM, Filip Pizlo fpi...@apple.com wrote: Interesting. What privileges, if any, would you propose 'emeritus reviewers' to have? -Filip On Apr 9, 2013, at 2:54 PM, Dmitry Titov dim...@chromium.org wrote: How about creating an 'emeritus reviewer' status (no r+ power) and let people *voluntarily* move themselves to this status? I bet a lot of 'inactive reviewers' would do that, since everybody understands the issue of getting out of sync with current code base. It may have different vibe though than figuring some automatic time-based enforcement system... As an added bonus, this gives such people a good way to avoid being asked to review a patch for a colleague while keeping some ties with the project... On Mon, Apr 8, 2013 at 3:34 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. We sometimes get low-quality drive-by reviews even from people who are active at the time. I feel like that's not the right basis for the time cutoff. If we do have a sunset period, we should think about it in terms of how long it takes to be so out of touch with the current state of the project that there's little chance you can give a useful review. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
I don't know if this is in line with what you and Dmitry were thinking, but here's what I like about a symbolic emeritus status: it takes care of the possible drive-by review problem while also continuing to recognize the person within the project. It's symbolic, and it feels nicer than just wiping them from the system. The fact that it serves the purpose of supporting lookup (your point (2)) is good, also. -Filip On Apr 10, 2013, at 12:37 AM, Eric Seidel e...@webkit.org wrote: Unrelated to Dmitry's suggestion, but since I brought up emeritus contributors earlier in the thread, I should explain my usage. The emeritus class proposed in the ancient webkit-reviewers thread about sunsetting was simply to answer the fact that committers.py has two purposes: 1. It exists as a public Access Control List (ACL) for svn.webkit.org (since I know of no other public ACL). 2. It exists as a way for our tools to lookup Contributor objects based on name, irc nick, svn email etc. emeritus contributors (in my original proposal on webkit-reviewers years ago) only exist to serve the second purpose, not the first. This would be similar to the Contributor and Account superclasses which have (since that old thread) been added to committers.py to allow committers.py to list non-commiters and even bots which we might want to have in our CC list, but not give any privileges to. Again, my thoughts on this may bear no reflection to what Dmitry had in mind with his use of the word emeritus, so I should let him speak! On Tue, Apr 9, 2013 at 11:39 PM, Filip Pizlo fpi...@apple.com wrote: Interesting. What privileges, if any, would you propose 'emeritus reviewers' to have? -Filip On Apr 9, 2013, at 2:54 PM, Dmitry Titov dim...@chromium.org wrote: How about creating an 'emeritus reviewer' status (no r+ power) and let people *voluntarily* move themselves to this status? I bet a lot of 'inactive reviewers' would do that, since everybody understands the issue of getting out of sync with current code base. It may have different vibe though than figuring some automatic time-based enforcement system... As an added bonus, this gives such people a good way to avoid being asked to review a patch for a colleague while keeping some ties with the project... On Mon, Apr 8, 2013 at 3:34 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. We sometimes get low-quality drive-by reviews even from people who are active at the time. I feel like that's not the right basis for the time cutoff. If we do have a sunset period, we should think about it in terms of how long it takes to be so out of touch with the current state of the project that there's little chance you can give a useful review. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
If we create an emeritus class in committers.py, we also have a whole bunch of old (long-webkit-retired) Apple committers/reviewers (ken, vicki, cblu, gramps, etc.) which should go there. :) Then the tools (including validate-committer-lists) would be less confused by their presence in ChangeLogs and commit messages. On Wed, Apr 10, 2013 at 12:41 AM, Filip Pizlo fpi...@apple.com wrote: I don't know if this is in line with what you and Dmitry were thinking, but here's what I like about a symbolic emeritus status: it takes care of the possible drive-by review problem while also continuing to recognize the person within the project. It's symbolic, and it feels nicer than just wiping them from the system. The fact that it serves the purpose of supporting lookup (your point (2)) is good, also. -Filip On Apr 10, 2013, at 12:37 AM, Eric Seidel e...@webkit.org wrote: Unrelated to Dmitry's suggestion, but since I brought up emeritus contributors earlier in the thread, I should explain my usage. The emeritus class proposed in the ancient webkit-reviewers thread about sunsetting was simply to answer the fact that committers.py has two purposes: 1. It exists as a public Access Control List (ACL) for svn.webkit.org (since I know of no other public ACL). 2. It exists as a way for our tools to lookup Contributor objects based on name, irc nick, svn email etc. emeritus contributors (in my original proposal on webkit-reviewers years ago) only exist to serve the second purpose, not the first. This would be similar to the Contributor and Account superclasses which have (since that old thread) been added to committers.py to allow committers.py to list non-commiters and even bots which we might want to have in our CC list, but not give any privileges to. Again, my thoughts on this may bear no reflection to what Dmitry had in mind with his use of the word emeritus, so I should let him speak! On Tue, Apr 9, 2013 at 11:39 PM, Filip Pizlo fpi...@apple.com wrote: Interesting. What privileges, if any, would you propose 'emeritus reviewers' to have? -Filip On Apr 9, 2013, at 2:54 PM, Dmitry Titov dim...@chromium.org wrote: How about creating an 'emeritus reviewer' status (no r+ power) and let people *voluntarily* move themselves to this status? I bet a lot of 'inactive reviewers' would do that, since everybody understands the issue of getting out of sync with current code base. It may have different vibe though than figuring some automatic time-based enforcement system... As an added bonus, this gives such people a good way to avoid being asked to review a patch for a colleague while keeping some ties with the project... On Mon, Apr 8, 2013 at 3:34 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. We sometimes get low-quality drive-by reviews even from people who are active at the time. I feel like that's not the right basis for the time cutoff. If we do have a sunset period, we should think about it in terms of how long it takes to be so out of touch with the current state of the project that there's little chance you can give a useful review. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Wed, Apr 10, 2013 at 1:46 AM, Eric Seidel e...@webkit.org wrote: If we create an emeritus class in committers.py, we also have a whole bunch of old (long-webkit-retired) Apple committers/reviewers (ken, vicki, cblu, gramps, etc.) which should go there. :) Then the tools (including validate-committer-lists) would be less confused by their presence in ChangeLogs and commit messages. I like this. Shouldn't be hard to add a suggest-emeriti command to webkitpy. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
How about creating an 'emeritus reviewer' status (no r+ power) and let people *voluntarily* move themselves to this status? I bet a lot of 'inactive reviewers' would do that, since everybody understands the issue of getting out of sync with current code base. It may have different vibe though than figuring some automatic time-based enforcement system... As an added bonus, this gives such people a good way to avoid being asked to review a patch for a colleague while keeping some ties with the project... On Mon, Apr 8, 2013 at 3:34 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. We sometimes get low-quality drive-by reviews even from people who are active at the time. I feel like that's not the right basis for the time cutoff. If we do have a sunset period, we should think about it in terms of how long it takes to be so out of touch with the current state of the project that there's little chance you can give a useful review. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Sunday 7. April 2013 18.27.14 Benjamin Poulain wrote: On Sun, Apr 7, 2013 at 6:07 PM, Dirk Schulze dschu...@adobe.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. The question is still how you measure active reviewers/contributors? Is it enough to comment on bugs? Real reviews? Must there be at least one r+ in this time? Is an actual commit required? What do we gain from reverting reviewer ship/ committer ship? There is a problem of people not contributing for a while, not familiar with the current code base, who come and review things for their colleagues. There are bad ideas accepted by reviewers who are not very active on the project. And instead of addressing these reviewers directly we are trying to introduce a process to automate this, avoid the confrontation, hope that reviewers accepting bad ideas today will instead expire in the future. It appears to me that this approach is based on the assumption that trust fades away over time. Naturally this perception may differ from person to person. I have friends from school that I meet maybe once every two years. Oddly I still do trust them as much as I did before we went different ways in our life. The trust was earned at some point and for me it only changes on a per- incident basis. Simon P.S.: I'm all in favour of locking unused SVN accounts for security reasons, similar to how many of us corporate email users have to change our passwords every X months. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Mon, Apr 8, 2013 at 4:27 AM, Benjamin Poulain benja...@webkit.orgwrote: On Sun, Apr 7, 2013 at 6:07 PM, Dirk Schulze dschu...@adobe.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. The question is still how you measure active reviewers/contributors? Is it enough to comment on bugs? Real reviews? Must there be at least one r+ in this time? Is an actual commit required? What do we gain from reverting reviewer ship/ committer ship? There is a problem of people not contributing for a while, not familiar with the current code base, who come and review things for their colleagues. There are bad ideas accepted by reviewers who are not very active on the project. I don't really see the big deal with revoking reviewer rights. If you come back to the project, make a few good patches and show a good understanding of the code base, you just get the rights back. The owner system with WebKit2 is avoiding this problem in an elegant way. It is effectively enforcing two reviews for most patches (one reviewer + one review from a owner). As a result, the quality of patches in WebKit2 has increased appreciably. Elegant is a bold claim (at least how it is implemented on WK2). There are examples of patches waiting for owners review/comments for months (even though the patch was already pre-reviewed by someone else). I suppose we also need another thread to discuss this issue... What are your concerns exactly? Benjamin PS: Maybe we should have this thread on the reviewer mailing list? Please, let's keep this at least to the committers list. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Mon, Apr 8, 2013 at 1:45 AM, Simon Hausmann simon.hausm...@digia.comwrote: And instead of addressing these reviewers directly we are trying to introduce a process to automate this, avoid the confrontation, hope that reviewers accepting bad ideas today will instead expire in the future. It appears to me that this approach is based on the assumption that trust fades away over time. Naturally this perception may differ from person to person. In my opinion, reviews are not trust affair, they are a technical decision. You seem to think people intentionally review bad thinks, I don't agree. When I mess up a review, it is because of the illusion of knowledge. I believed I knew enough about a subject to review a patch, but my knowledge was outdated or erroneous. With time, this problem becomes worse. I believe I know the code, but I only know the past version of the code. With hundreds of patches a day, I think not contributing for 2 years means you have an outdated view of the code. ...And I am also interested in seeing if such a policy would become a stimulus for people to review more regularly. :) Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Mon, Apr 8, 2013 at 10:36 AM, Benjamin Poulain benja...@webkit.orgwrote: On Mon, Apr 8, 2013 at 1:45 AM, Simon Hausmann simon.hausm...@digia.comwrote: And instead of addressing these reviewers directly we are trying to introduce a process to automate this, avoid the confrontation, hope that reviewers accepting bad ideas today will instead expire in the future. It appears to me that this approach is based on the assumption that trust fades away over time. Naturally this perception may differ from person to person. In my opinion, reviews are not trust affair, they are a technical decision. You seem to think people intentionally review bad thinks, I don't agree. When I mess up a review, it is because of the illusion of knowledge. I believed I knew enough about a subject to review a patch, but my knowledge was outdated or erroneous. With time, this problem becomes worse. I believe I know the code, but I only know the past version of the code. With hundreds of patches a day, I think not contributing for 2 years means you have an outdated view of the code. +1 to that. When I make bad reviews (I do!), I don't often realize it until bugs are filed for regressions or some other more knowledgable reviewers comment on the bug, pointing out flaws in the patch. In a way, this is an unsolvable problem because nobody can possibly know with a 100% certainly if you're qualified to review a patch or not. I've reviewed some patches that are clearly outside of my expertise at times because there were no active reviewers in that area, etc... It's a hard judgement call. On one hand, we don't want to block patches forever especially it's a crash or security vulnerability fix. On the other hand, we shouldn't be reviewing patches just because we want to be nice to our colleague. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. We sometimes get low-quality drive-by reviews even from people who are active at the time. I feel like that's not the right basis for the time cutoff. If we do have a sunset period, we should think about it in terms of how long it takes to be so out of touch with the current state of the project that there's little chance you can give a useful review. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
+1 IMO, as Dirk suggested, the deactivation of the account is more reasonable unless the reviewership or committership is revoked, Gyuyoung On Fri, Apr 5, 2013 at 4:10 PM, Dirk Schulze dschu...@adobe.com wrote: Sent from my iPhone On Apr 5, 2013, at 12:00 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, Apr 4, 2013 at 11:53 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: I am not sure this is really needed. People sometimes disappear from working on trunk for extended periods of time due to internal products and downstream branches. It has happened multiple times to me. That doesn't mean that I won't come back and start working upstream later. Also it could be that some people working on Blink would like to contribute to WebKit in their spare time or in the future again. Part of being a reviewer is also knowing what and when to review, so I am not sure there really is an issue here. I'm not too concerned about the reviwership but more about committership from a security point of view. I don't think a lot of committers are going to monitor their old SVN accounts and update passwords periodically. Having lots of inactive SVN accounts isn't that helpful. Maybe I didn't phrase it correctly, but I'm suggesting more of suspension so that we have a smaller attack surface for SVN credentials. Suggesting the deactivation of the SVN account is reasonable, as long as you get it back on request. Greetings Dirk - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. The question is still how you measure active reviewers/contributors? Is it enough to comment on bugs? Real reviews? Must there be at least one r+ in this time? Is an actual commit required? What do we gain from reverting reviewer ship/ committer ship? I can absolutely understand security concerns on unused accounts. But this is the case on active contributors as well. How many people did change their passwords after the first activation of SVN access? If we have security concerns, we should reset all passwords for all people with SVN accounts from time to time. Greetings, Dirk Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Sun, Apr 7, 2013 at 6:07 PM, Dirk Schulze dschu...@adobe.com wrote: On Apr 7, 2013, at 5:53 PM, Benjamin Poulain benja...@webkit.org wrote: On Sun, Apr 7, 2013 at 5:49 PM, Timothy Hatcher timo...@apple.com wrote: I think 6 months is fine for deactivating SVN accounts. And a full revoke of reviewer status after 2 years of no activity sounds reasonable to me. We could make it easier to get reviewer status again after a 2 year sunset if the person becomes active again and shows good judgment still. +1 to this. I think 2 years to revoke reviewer rights is too long. All the drive-by reviews that have caused problems were from reviewers that were inactive for less than 2 years. Nevertheless, 2 years is better than the current situation so it is a good start. The question is still how you measure active reviewers/contributors? Is it enough to comment on bugs? Real reviews? Must there be at least one r+ in this time? Is an actual commit required? What do we gain from reverting reviewer ship/ committer ship? There is a problem of people not contributing for a while, not familiar with the current code base, who come and review things for their colleagues. There are bad ideas accepted by reviewers who are not very active on the project. I don't really see the big deal with revoking reviewer rights. If you come back to the project, make a few good patches and show a good understanding of the code base, you just get the rights back. The owner system with WebKit2 is avoiding this problem in an elegant way. It is effectively enforcing two reviews for most patches (one reviewer + one review from a owner). As a result, the quality of patches in WebKit2 has increased appreciably. What are your concerns exactly? Benjamin PS: Maybe we should have this thread on the reviewer mailing list? ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Sun, Apr 7, 2013 at 7:27 PM, Benjamin Poulain benja...@webkit.orgwrote: I don't really see the big deal with revoking reviewer rights. If you come back to the project, make a few good patches and show a good understanding of the code base, you just get the rights back. This seems rather subjective criteria. What is a good understanding of the code base? Does that mean one module, a collection of modules in some namespace, or the entire breadth of the source tree? What constitutes a good patch? It might be interesting to learn which and how many reviewers would be withdrawn if a 6 month window were invoked now. For example, how many Apple reviewers would go away? ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Sunsetting committership and reviewership
Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Any thoughts or opinions? - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
I am not sure this is really needed. People sometimes disappear from working on trunk for extended periods of time due to internal products and downstream branches. It has happened multiple times to me. That doesn't mean that I won't come back and start working upstream later. Also it could be that some people working on Blink would like to contribute to WebKit in their spare time or in the future again. Part of being a reviewer is also knowing what and when to review, so I am not sure there really is an issue here. Cheers Kenneth On Fri, Apr 5, 2013 at 8:19 AM, Ryosuke Niwa rn...@webkit.org wrote: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Any thoughts or opinions? - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev -- Kenneth Rohde Christiansen Senior Engineer, WebKit, Qt, EFL Phone +45 4294 9458 / E-mail kenneth at webkit.org ﹆﹆﹆ ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Apr 5, 2013, at 2:19 AM, Ryosuke Niwa rn...@webkit.org wrote: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Any thoughts or opinions? My third-party perspective: This might make sense for reviewer status which is more about project leadership/direction but perhaps not for committer status which is more about proven good judgement and experience. I don't think the WebKit code base evolves so unexpectedly that 6 months invalidates the qualifications for committer status. Also consider: 1. Removing committers is actually more work than not removing them -- you now need to track start dates and run jobs to identify expired people. You also potentially need warnings, a review process in case of mistakes, etc. 2. People employed for WebKit development might be absent 6 months for totally valid reasons like maternity/paternity leave. Regards, Marshall - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
Am 05.04.2013 um 08:19 schrieb Ryosuke Niwa rn...@webkit.org: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Hm, that sounds really harsh - at least if the person is still within the WebKit project. I personally couldn't land a patch the last months as I'm busy with my thesis, but I'm clearly still qualified for SVG reviews and discussions, no? I follow development closely, even if not writing patches on my own at the moment. Cheers, Niko ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Apr 4, 2013, at 11:19 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Any thoughts or opinions? This seems reactionary. We never had an activity counter before. Actually, I know WebKit reviewers who stopped working on the project for 2 years and restarted the activity later. Even with a fast growing community and an always changing code base, the expertise was still good enough to continue where stopped right away. On the controversy, it may leave people at the Blink project who want to switch back to WebKit later and even more harm is done to the WebKit project. I object to such a proposal. Greetings Dirk - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Thu, Apr 4, 2013 at 11:53 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: I am not sure this is really needed. People sometimes disappear from working on trunk for extended periods of time due to internal products and downstream branches. It has happened multiple times to me. That doesn't mean that I won't come back and start working upstream later. Also it could be that some people working on Blink would like to contribute to WebKit in their spare time or in the future again. Part of being a reviewer is also knowing what and when to review, so I am not sure there really is an issue here. I'm not too concerned about the reviwership but more about committership from a security point of view. I don't think a lot of committers are going to monitor their old SVN accounts and update passwords periodically. Having lots of inactive SVN accounts isn't that helpful. Maybe I didn't phrase it correctly, but I'm suggesting more of suspension so that we have a smaller attack surface for SVN credentials. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Thu, Apr 4, 2013 at 11:56 PM, Nikolas Zimmermann zimmerm...@physik.rwth-aachen.de wrote: Am 05.04.2013 um 08:19 schrieb Ryosuke Niwa rn...@webkit.org: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Hm, that sounds really harsh - at least if the person is still within the WebKit project. I personally couldn't land a patch the last months as I'm busy with my thesis, but I'm clearly still qualified for SVG reviews and discussions, no? I follow development closely, even if not writing patches on my own at the moment. Would something like one year be a reasonable timeframe? I'm not suggesting that you have to go through non-committer status and wait for committer/reviewer nominations again once you passed that threshold. It's more about temporarily suspending accounts and removing people from committers.py to better access control things. If they're coming back to WebKit, they can request their committerships and reviewerships reinstated. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
Neither here nor there, but... I had interest in sunsetting committers/reviewers in the past. There are loads of folks listed in committers.py who haven't committed or reviewed in 5+ years. I believe there are some old threads on webkit-reviewers about such. I believe the timeout for sunsetting discussed in those old threads was more along the lines of 2-years. I should note that committers.py has some historical uses (for associating committers with commits), so you might want to consider using an emeritus section instead of removing entries. Best of luck. On Fri, Apr 5, 2013 at 12:05 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, Apr 4, 2013 at 11:56 PM, Nikolas Zimmermann zimmerm...@physik.rwth-aachen.de wrote: Am 05.04.2013 um 08:19 schrieb Ryosuke Niwa rn...@webkit.org: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Hm, that sounds really harsh - at least if the person is still within the WebKit project. I personally couldn't land a patch the last months as I'm busy with my thesis, but I'm clearly still qualified for SVG reviews and discussions, no? I follow development closely, even if not writing patches on my own at the moment. Would something like one year be a reasonable timeframe? I'm not suggesting that you have to go through non-committer status and wait for committer/reviewer nominations again once you passed that threshold. It's more about temporarily suspending accounts and removing people from committers.py to better access control things. If they're coming back to WebKit, they can request their committerships and reviewerships reinstated. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
Sorry. Wrong address again. On Fri, Apr 5, 2013 at 12:09 AM, Eric Seidel esei...@google.com wrote: Neither here nor there, but... I had interest in sunsetting committers/reviewers in the past. There are loads of folks listed in committers.py who haven't committed or reviewed in 5+ years. I believe there are some old threads on webkit-reviewers about such. I believe the timeout for sunsetting discussed in those old threads was more along the lines of 2-years. I should note that committers.py has some historical uses (for associating committers with commits), so you might want to consider using an emeritus section instead of removing entries. Best of luck. On Fri, Apr 5, 2013 at 12:05 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, Apr 4, 2013 at 11:56 PM, Nikolas Zimmermann zimmerm...@physik.rwth-aachen.de wrote: Am 05.04.2013 um 08:19 schrieb Ryosuke Niwa rn...@webkit.org: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Hm, that sounds really harsh - at least if the person is still within the WebKit project. I personally couldn't land a patch the last months as I'm busy with my thesis, but I'm clearly still qualified for SVG reviews and discussions, no? I follow development closely, even if not writing patches on my own at the moment. Would something like one year be a reasonable timeframe? I'm not suggesting that you have to go through non-committer status and wait for committer/reviewer nominations again once you passed that threshold. It's more about temporarily suspending accounts and removing people from committers.py to better access control things. If they're coming back to WebKit, they can request their committerships and reviewerships reinstated. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
Sent from my iPhone On Apr 5, 2013, at 12:00 AM, Ryosuke Niwa rn...@webkit.orgmailto:rn...@webkit.org wrote: On Thu, Apr 4, 2013 at 11:53 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.commailto:kenneth.christian...@gmail.com wrote: I am not sure this is really needed. People sometimes disappear from working on trunk for extended periods of time due to internal products and downstream branches. It has happened multiple times to me. That doesn't mean that I won't come back and start working upstream later. Also it could be that some people working on Blink would like to contribute to WebKit in their spare time or in the future again. Part of being a reviewer is also knowing what and when to review, so I am not sure there really is an issue here. I'm not too concerned about the reviwership but more about committership from a security point of view. I don't think a lot of committers are going to monitor their old SVN accounts and update passwords periodically. Having lots of inactive SVN accounts isn't that helpful. Maybe I didn't phrase it correctly, but I'm suggesting more of suspension so that we have a smaller attack surface for SVN credentials. Suggesting the deactivation of the SVN account is reasonable, as long as you get it back on request. Greetings Dirk - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.orgmailto:webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Apr 4, 2013, at 11:19 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Any thoughts or opinions? Not a completely unreasonable idea, but I feel 3-6 months is way too short. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Sunsetting committership and reviewership
On Fri, Apr 5, 2013 at 12:21 AM, Maciej Stachowiak m...@apple.com wrote: On Apr 4, 2013, at 11:19 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, This is somewhat related to the bulk move of Chromium-WebKit contributors to Blink, but we might want to consider sunsetting/expiring committership and reviewership. I'm thinking of something like expiring committership/reviwership of a person after the person didn't have any SVN activities for 3 or 6 months. Any thoughts or opinions? Not a completely unreasonable idea, but I feel 3-6 months is way too short. How about 2 years as Eric cited? - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
