caml-list - the Caml user's mailing list
 help / color / mirror / Atom feed
From: Yotam Barnoy <yotambarnoy@gmail.com>
To: Ocaml Mailing List <caml-list@inria.fr>
Subject: [Caml-list] Recent hooks design and general github PR issues
Date: Tue, 19 Jul 2016 11:08:56 -0400	[thread overview]
Message-ID: <CAN6ygOn7DQw35jTRxHFda5keLFgfJ7rE3dBQ7ezNJarK8L2rxg@mail.gmail.com> (raw)

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

             reply	other threads:[~2016-07-19 15:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-19 15:08 Yotam Barnoy [this message]
2016-07-19 15:36 ` Fabrice Le Fessant
2016-07-19 16:23   ` Yotam Barnoy
2016-07-19 17:28     ` Fabrice Le Fessant
2016-07-19 20:21 ` Alain Frisch
2016-07-21 12:09   ` Goswin von Brederlow
2016-07-21 12:38     ` Fabrice Le Fessant
2016-07-21 13:37       ` Hendrik Boom

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAN6ygOn7DQw35jTRxHFda5keLFgfJ7rE3dBQ7ezNJarK8L2rxg@mail.gmail.com \
    --to=yotambarnoy@gmail.com \
    --cc=caml-list@inria.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).