Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [websocket-dev] [External] : Re: Removing restrictions on direct commits
  • From: Jan Supol <jan.supol@xxxxxxxxxx>
  • Date: Tue, 9 Nov 2021 23:48:30 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=xazcdQTaoC9q6QOv5wNQvcLxVtKXmPAN4NlyJFU4RE4=; b=hXiAk/uhuCWfk83YDtqKRHLhsDVwN87st9KWlbcsCDLQ6cLaBz6qENO8hosIb2z47NQ+PDaZeITtpeby73hVIvs1LC0Rj4Nhn12XCJGGGVLQ42DU5JLK33G6JafIPkLxw37FESaNNLliIwsfMx7FkoMRv+wKJmk5s43ubS3IrZTlVQLQFo4xRzd3/hA3C6PrM60jDWTvwNnkTvXYQuveYLhyWfjueT6TW20I35pS89YOnFap624cEW6Ht8kK78bkybky9IIAdlAd0yIyVv4g8jfiA52dLUUbDw0EK8U8xBU8uUfkIyLgKLeMVq1YtuFWop98ogpgn4E/68cc5HxcYg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bEXleq02uQmU66Vl0MkaDekruozA/KA2x1cxXEMQTXgxN9onqPYUKXMdyiwAepCTwQ4jRreQPWdHmlVIY7CgOSE6BwHVIg/Uieu98ehBAliCnS+fWBFsnH/lD4lN2BdzuQpIETRF3N2/d/Q8p3SS+CPb9FFNTklk32sbE7URFaYDF1BtEWDmyPDZNV8CCpmU7MvBZA7z7oy/0Gdgaxm8xHxdMF06UBx1Jh2CXDuPr4C0c80IYImtZjzoNO/95mCYUIeED2KOgPUQ9hiubpCPV7CkJsGi+Z1E3HZ5BhVtpEAhOXc98+WQHCYzBl7Hg5e0ZgkSXxZyYTIlaMQ0RSTKFA==
  • Delivered-to: websocket-dev@xxxxxxxxxxx
  • List-archive: <https://www.eclipse.org/mailman/private/websocket-dev/>
  • List-help: <mailto:websocket-dev-request@eclipse.org?subject=help>
  • List-subscribe: <https://www.eclipse.org/mailman/listinfo/websocket-dev>, <mailto:websocket-dev-request@eclipse.org?subject=subscribe>
  • List-unsubscribe: <https://www.eclipse.org/mailman/options/websocket-dev>, <mailto:websocket-dev-request@eclipse.org?subject=unsubscribe>
  • Suggested_attachment_session_id: 3178f2bb-ab71-73f1-83f8-a98cd9eb5938
  • Thread-index: AQHX0dJEpjHyNTQqi0+gD5g8mKk5zKv1JkGAgAAAwzyAAA18AIAGpyM+
  • Thread-topic: [websocket-dev] [External] : Re: Removing restrictions on direct commits

Mark,
I realize that from the very majority you drive the WebSocket Spec forward, and the WebSocket community appreciates that. 

I understand that the WebSocket committer number is lower, and you wanted to be able to make small fast changes. But the direct commits are not the best way to solve it. With force push, it is not only possible to add commits, but it is also possible to remove commits. The PRs prevent that. 

Anyway, as for why, I'd assume it would be a good reason that committers did not want direct PRs.

Thanks,
Jan


From: websocket-dev <websocket-dev-bounces@xxxxxxxxxxx> on behalf of Mark Thomas <markt@xxxxxxxxxx>
Sent: Friday, November 5, 2021 6:39 PM
To: websocket-dev@xxxxxxxxxxx <websocket-dev@xxxxxxxxxxx>
Subject: Re: [websocket-dev] [External] : Re: Removing restrictions on direct commits
 
On 05/11/2021 16:58, Jan Supol wrote:
> Mark,
> I saw 3 emails from people saying that they want to keep PRs
> requirements. I did not see any that supported your wish to remove them.
> Yet, you step ahead.

I intended to keep the technical control that required PRs but that
wasn't possible (as far as I could see) with GitHub's permission scheme.
Therefore the options were a) retain the existing technical controls or
b) remove the techincal controls and rely on social controls. The latter
approach seemed reasonable so I went ahead.

The pool of WebSocket committers providing PRs is small. As far as I can
tell, every PR for WebSocket from a committer in the last 12 months has
been from me. I think it is likely that social controls will be
effective. If they are not, we can always revisit this change.

> Social requirements are not enough, since direct push-force to master
> can be a very dangerous thing,

Can you explain this please. EL has been working this way without any
issues I am aware of since creation. What is the risk that you consider
dangerous? What can go wrong? This is version control after all. In the
worst case, we simply have to revert a commit.

> and hard to trace the origin.

Can you expand on this? We know who made the commit and the commit
message should provide sufficient explanation. Anything substantive will
be via PR anyway. We are only talking about relatively trivial commits
(like the ones in #379 and #380).

What do you see as the benefits of #379 and #380 being via PR rather
than a direct commit?

> I expressed what I think about it already.

You have expressed the what but not the why and I'd like to understand
the why because, at the moment, I don't understand your position.

> Your idea of good faith is not good enough; everyone can make a mistake
> and the PRs are to eliminate them.

PRs don't eliminate mistakes. The handful of copyright fixes I've just
applied demonstrate that.

In my experience, across a wide range of OSS projects, using technical
controls rather than social controls has a stifling effect on
the community. Given the relatively low levels of activity in the
project I view anything the reduces friction as a good thing.

There are lots of examples of us using PRs to discuss potential changes.
#356 is a good example where it looks like the proposal is going to be
dropped entirely in favour of a better option.

What mistakes are you expecting PRs to eliminate?

Mark


>
> Thanks,
> Jan
> ------------------------------------------------------------------------
> *From:* websocket-dev <websocket-dev-bounces@xxxxxxxxxxx> on behalf of
> Mark Thomas <markt@xxxxxxxxxx>
> *Sent:* Friday, November 5, 2021 5:48 PM
> *To:* websocket developer discussions <websocket-dev@xxxxxxxxxxx>
> *Subject:* Re: [websocket-dev] [External] : Re: Removing restrictions on
> direct commits
> All,
>
> Now I have admin access it appears that if PRs are required then so are
> reviews. We can't require PRs without the review. Therefore I am going
> to remove the PR requirement.
>
> The social requirement that substantive changes must be via PR and must
> allow time for community review remains.
>
> I'll try and make all my changes via PR but a few trivial fixes may end
> up going directly to master.
>
> Mark
>
>
> On 04/11/2021 23:18, Ed Bratt wrote:
>> When reviewing changes further down the road, I find it easier to review
>> via PRs. I understand you can see the direct commits as well, but in my
>> perspective, these are harder to follow and unwind.
>>
>> My preference -- and this is mostly as a reader -- is that projects
>> adopt a convention to use PRs, even for small changes -- even if the
>> submitter is just going to turn right around and hit the merge button.
>>
>> My opinion, that's all.
>>
>> -- Ed
>>
>> On 11/4/2021 1:48 PM, Mark Thomas wrote:
>>> As a first step, I have opened an issue to make the project leads
>>> admins - as we should according to the Eclipse Handbook. With admin
>>> karma, we should then be able to make changes to the review requirements.
>>>
>>> Mark
>>>
>>>
>>> On 02/11/2021 16:29, Mark Thomas wrote:
>>>> All,
>>>>
>>>> In approx 24 hours time I intend to request that the branch
>>>> restrictions that prevent committers committing directly to the
>>>> master branch and those that require every PR to be reviewed before
>>>> merge are removed.
>>>>
>>>> My reasoning is as follows:
>>>> - I have seen the benefits of these restrictions not being present in EL
>>>> - I'm expecting a number of non-substantive changes will be required to
>>>>    successfully complete the release process and PR + review for all of
>>>>    them will significantly slow us down
>>>> - Committers are perfectly capable of determining which changes need a
>>>>    PR and review and which can be made directly - and if they get it
>>>>    wrong changes can easily be reverted
>>>>
>>>> I was intending to propose this change after the Jakarta 10 release
>>>> but on reflection, I think the sooner, the better.
>>>>
>>>> Thoughts? Comments? Objections?
>>>>
>>>> Mark
>>>> _______________________________________________
>>>> websocket-dev mailing list
>>>> websocket-dev@xxxxxxxxxxx
>>>> To unsubscribe from this list, visit
>>>> https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!ZXaPJ_2lrxntT4crrTbge_DgyBpJUYhh2hOFgetAtSlIXV4Ra_qRKksqY9T8pi0$
> <https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!ZXaPJ_2lrxntT4crrTbge_DgyBpJUYhh2hOFgetAtSlIXV4Ra_qRKksqY9T8pi0$>
>
>>>
>>>
>>>
>>> _______________________________________________
>>> websocket-dev mailing list
>>> websocket-dev@xxxxxxxxxxx
>>> To unsubscribe from this list, visit
>>> https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!ZXaPJ_2lrxntT4crrTbge_DgyBpJUYhh2hOFgetAtSlIXV4Ra_qRKksqY9T8pi0$
> <https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!ZXaPJ_2lrxntT4crrTbge_DgyBpJUYhh2hOFgetAtSlIXV4Ra_qRKksqY9T8pi0$>
>
>>
>>
>
> _______________________________________________
> websocket-dev mailing list
> websocket-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit
> https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!aE6EGNbfFFvU6s1M10dA24CFRYmgf2yFA7YivHmg-GSukZ6Gl-IvOnFZReOPjQRQ$
> <https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!aE6EGNbfFFvU6s1M10dA24CFRYmgf2yFA7YivHmg-GSukZ6Gl-IvOnFZReOPjQRQ$>
>
>
> _______________________________________________
> websocket-dev mailing list
> websocket-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!coubJkvEI4EKK6SprL0sNT0hB8mcN2rndT2-DSe-Aqf-6LdIj183lbGj7uSSOm4z$
>

_______________________________________________
websocket-dev mailing list
websocket-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/websocket-dev__;!!ACWV5N9M2RV99hQ!coubJkvEI4EKK6SprL0sNT0hB8mcN2rndT2-DSe-Aqf-6LdIj183lbGj7uSSOm4z$

Back to the top