From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Original-To: caml-list@sympa.inria.fr Delivered-To: caml-list@sympa.inria.fr Received: from mail2-relais-roc.national.inria.fr (mail2-relais-roc.national.inria.fr [192.134.164.83]) by sympa.inria.fr (Postfix) with ESMTPS id E5C6F7EF1E for ; Tue, 19 Jul 2016 17:37:03 +0200 (CEST) IronPort-PHdr: 9a23:Vcsw6BRWd7Hu/njExNPNKd0JItpsv+yvbD5Q0YIujvd0So/mwa65YBCN2/xhgRfzUJnB7Loc0qyN4vimBDVLscrJmUtBWaQEbwUCh8QSkl5oK+++Imq/EsTXaTcnFt9JTl5v8iLzG0FUHMHjew+a+SXqvnYsExnyfTB4Ov7yUtaLyZ/mj6bvotaDP01hv3mUWftKNhK4rAHc5IE9oLBJDeIP8CbPuWZCYO9MxGlldhq5lhf44dqsrtY4q3wD89pozcNLUL37cqIkVvQYSW1+ayFm0vb2rgHORhej4X4VU2Ne0kYZQluN0Bavb5Dtuy6ynONn3i6LdZnnSqw9XD6r9aFsWTfnjS4GM3gy92SBzoRXh6tepFqErh17wojbKKWUL+Y2KqjUeNdfQWtaQu5QUTZAC8Wydd1cIfAGOLN2poPnplAD5T+zTSehH/jmzCMA0lH/17c72OlnNQzx0gE7BNsIrFzVqs/0PeEcS7bmn+Hz0TzfYqYOin/G44/Sf0Vk/KiB Authentication-Results: mail2-smtp-roc.national.inria.fr; spf=None smtp.pra=Fabrice.Le_fessant@inria.fr; spf=Pass smtp.mailfrom=fabrissimo@gmail.com; spf=None smtp.helo=postmaster@mail-wm0-f50.google.com Received-SPF: None (mail2-smtp-roc.national.inria.fr: no sender authenticity information available from domain of Fabrice.Le_fessant@inria.fr) identity=pra; client-ip=74.125.82.50; receiver=mail2-smtp-roc.national.inria.fr; envelope-from="fabrissimo@gmail.com"; x-sender="Fabrice.Le_fessant@inria.fr"; x-conformance=sidf_compatible Received-SPF: Pass (mail2-smtp-roc.national.inria.fr: domain of fabrissimo@gmail.com designates 74.125.82.50 as permitted sender) identity=mailfrom; client-ip=74.125.82.50; receiver=mail2-smtp-roc.national.inria.fr; envelope-from="fabrissimo@gmail.com"; x-sender="fabrissimo@gmail.com"; x-conformance=sidf_compatible; x-record-type="v=spf1" Received-SPF: None (mail2-smtp-roc.national.inria.fr: no sender authenticity information available from domain of postmaster@mail-wm0-f50.google.com) identity=helo; client-ip=74.125.82.50; receiver=mail2-smtp-roc.national.inria.fr; envelope-from="fabrissimo@gmail.com"; x-sender="postmaster@mail-wm0-f50.google.com"; x-conformance=sidf_compatible X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0DkAAAGSI5XhjJSfUpchAgNfKcwhSCHFoUEghyHKjgUAQEBAQEBAQERAQEBCAsLCSEvgjIEARKCFAEEARIRSxALCwsaCBUCAiEBEgEFAQoSBgESEhCHdAMPCA6gcoEyPjGLO4lfAwqEKiUQimeCQ4I8gkKCWgWGVgySDjSGE4Yxgh6Ba06EC4hziCWGOjCBDw8PgkEcgUw8MoglAQEB X-IPAS-Result: A0DkAAAGSI5XhjJSfUpchAgNfKcwhSCHFoUEghyHKjgUAQEBAQEBAQERAQEBCAsLCSEvgjIEARKCFAEEARIRSxALCwsaCBUCAiEBEgEFAQoSBgESEhCHdAMPCA6gcoEyPjGLO4lfAwqEKiUQimeCQ4I8gkKCWgWGVgySDjSGE4Yxgh6Ba06EC4hziCWGOjCBDw8PgkEcgUw8MoglAQEB X-IronPort-AV: E=Sophos;i="5.28,390,1464645600"; d="scan'208,217";a="227177754" Received: from mail-wm0-f50.google.com ([74.125.82.50]) by mail2-smtp-roc.national.inria.fr with ESMTP/TLS/AES128-GCM-SHA256; 19 Jul 2016 17:37:03 +0200 Received: by mail-wm0-f50.google.com with SMTP id i5so31296773wmg.0 for ; Tue, 19 Jul 2016 08:37:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=JgnChIRfAYBn6r1ADiDYdg5JFSL28UtLRQFNIpkI29M=; b=cxYbKn9BDObDNcCJvd84aOW+1jXTfDlGIt9RKYQUdQs2ny4M8/hqOMpBKquC4YEr41 ltt94OgNIHidxLZ6/YTFI3lCnQJbXBXUIRFkGCVY5h+UvpHTmMzW0y8crilD5WaF+/Ww 3d6tpEXrM3HnUlpO2mQ07j0kN6k1Jo7xTdLB4QpqLIFDzLqaXmf/aPCzhdCtsTnf0wv1 XalHCVaOOl9jkSImVlPHrTuWsws66DsTFvqEp3bVSIAVWrefiSiF9liXoFjKrrjTM58u mhiQm3bkra9XAFhuNyQff7Y6Wu0RuDucO2D+NLJw3qzqJPL0uT2uxa0FJkUjXPWLvI0z NaLg== X-Gm-Message-State: ALyK8tKKFb0J5ECzhzBPS66YFXupvACVoo0SvM8wgCZgUF5S5MyBfmRmDfcqJbZANWAV94MKrbuKA4A4ar75YQ== X-Received: by 10.28.218.67 with SMTP id r64mr5259200wmg.10.1468942622863; Tue, 19 Jul 2016 08:37:02 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Fabrice Le Fessant Date: Tue, 19 Jul 2016 15:36:53 +0000 Message-ID: To: Yotam Barnoy , Ocaml Mailing List Content-Type: multipart/alternative; boundary=001a114699923b34550537fedbeb Subject: Re: [Caml-list] Recent hooks design and general github PR issues --001a114699923b34550537fedbeb Content-Type: text/plain; charset=UTF-8 Hi, > several core members trying to voice concerns about the feature in one PR, while other member PRs were simply merged into the tree Could you be more specific ? I think I have taken the time to discuss all these PRs (for 20 days for 2 of them, 13 days for the other one), I have modified the PRs to follow the requests that were voiced (for example, by removing the default -fPIC) and they were all reviewed by other core developers. If you think some concerns have not been taken into account, please, tell me. > yet I have not seen it or its costs vs benefits being discussed fully in any particular place Probably because the cost vs benefit is very hard to measure in most PR: the benefit might be small for some users, and big for other ones; some features might be hard to understand and to use, yet one developer can use them to develop a tool that can be used by many users; some features might look of little use at the beginning, yet, once they are present, other users might find usages that were not foreseen in the PR discussion. --Fabrice On Tue, Jul 19, 2016 at 5:09 PM Yotam Barnoy wrote: > I would like to alert the list to a potential problem with github PRs, > and a recent example of such a problem. > > I've noticed that recently, there has been an insertion of a > particular feature into the compiler. This feature involves > potentially long-term commitment in terms of maintenance, and yet I > have not seen it or its costs vs benefits being discussed fully in any > particular place. I'm specifically talking about PRs > https://github.com/ocaml/ocaml/pull/648, > https://github.com/ocaml/ocaml/pull/668 and > https://github.com/ocaml/ocaml/pull/647. > > The problem with github PRs is that they allow you to insert features > piecemeal. This splits up the discussion across multiple PRs, making > it very difficult to have a discussion about the feature as a whole, > and making it seem like there is consensus when there might not be. > > As a rule, I recommend that any such large feature spanning multiple > PRs first require discussion either on the list or on mantis, to > concentrate discussion in one place. > > It may also be worthwhile to say that except for rare exceptions > (mostly bug fixes), PRs should not be merged by the same person who > authored them, as this makes the process seem biased and questionable. > > I don't know if this feature in itself is problematic, but I've seen > several core members trying to voice concerns about the feature in one > PR, while other member PRs were simply merged into the tree. In my > opinion, questions on sub-PRs of a feature should inhibit merging any > parts of said feature. > > This may be a false alert, but I think it is worth clarifying if it is > indeed one, and at the very least, the protocol should be set for the > future. > > -Yotam > > -- > Caml-list mailing list. Subscription management and archives: > https://sympa.inria.fr/sympa/arc/caml-list > Beginner's list: http://groups.yahoo.com/group/ocaml_beginners > Bug reports: http://caml.inria.fr/bin/caml-bugs > --001a114699923b34550537fedbeb Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi,

> =C2=A0 several core members tr= ying to voice concerns about the feature in one=C2=A0PR, while other member PRs were simply merged into the tree

Could you be more specific ? I think I have taken th= e time to discuss all these PRs (for 20 days for 2 of them, 13 days for the= other one), I have modified the PRs to follow the requests that were voice= d (for example, by removing the default -fPIC) and they were all reviewed b= y other core developers. If you think some concerns have not been taken int= o account, please, tell me.

> =C2=A0yet= I=C2=A0have not seen it or its cost= s vs benefits being discussed fully in any=C2=A0particular place

Probably because the cost vs benefit is very= hard to measure in most PR: the benefit might be small for some users, and= big for other ones; some features might be hard to understand and to use, = yet one developer can use them to develop a tool that can be used by many u= sers; some features might look of little use at the beginning, yet, once th= ey are present, other users might find usages that were not foreseen in the= PR discussion.

--Fabrice


On Tue, Jul 19, 2016 at 5= :09 PM Yotam Barnoy <yotambarno= y@gmail.com> wrote:
I would = like to alert the list to a potential problem with github PRs,
and a recent example of such a problem.

I've noticed that recently, there has been an insertion of a
particular feature into the compiler. This feature involves
potentially long-term commitment in terms of maintenance, and yet I
have not seen it or its costs vs benefits being discussed fully in any
particular place. I'm specifically talking about PRs
https://github.com/ocaml/ocaml/pull/648,
https://github.com/ocaml/ocaml/pull/668 and
https://github.com/ocaml/ocaml/pull/647.

The problem with github PRs is that they allow you to insert features
piecemeal. This splits up the discussion across multiple PRs, making
it very difficult to have a discussion about the feature as a whole,
and making it seem like there is consensus when there might not be.

As a rule, I recommend that any such large feature spanning multiple
PRs first require discussion either on the list or on mantis, to
concentrate discussion in one place.

It may also be worthwhile to say that except for rare exceptions
(mostly bug fixes), PRs should not be merged by the same person who
authored them, as this makes the process seem biased and questionable.

I don't know if this feature in itself is problematic, but I've see= n
several core members trying to voice concerns about the feature in one
PR, while other member PRs were simply merged into the tree. In my
opinion, questions on sub-PRs of a feature should inhibit merging any
parts of said feature.

This may be a false alert, but I think it is worth clarifying if it is
indeed one, and at the very least, the protocol should be set for the
future.

-Yotam

--
Caml-list mailing list.=C2=A0 Subscription management and archives:
https://sympa.inria.fr/sympa/arc/caml-list
Beginner's list: http://groups.yahoo.com/group/ocam= l_beginners
Bug reports: http://caml.inria.fr/bin/caml-bugs
--001a114699923b34550537fedbeb--