path: root/dd/1251fdea02a159ddd65f9af9d9ea167cb6b18d

Received: from sog-mx-3.v43.ch3.sourceforge.com ([172.29.43.193]
	helo=mx.sourceforge.net)
	by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76)
	(envelope-from <jgarzik@bitpay.com>) id 1Y0dbL-000485-V5
	for bitcoin-development@lists.sourceforge.net;
	Mon, 15 Dec 2014 21:54:47 +0000
Received-SPF: pass (sog-mx-3.v43.ch3.sourceforge.com: domain of bitpay.com
	designates 209.85.213.179 as permitted sender)
	client-ip=209.85.213.179; envelope-from=jgarzik@bitpay.com;
	helo=mail-ig0-f179.google.com; 
Received: from mail-ig0-f179.google.com ([209.85.213.179])
	by sog-mx-3.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128)
	(Exim 4.76) id 1Y0dbK-0007D4-Iw
	for bitcoin-development@lists.sourceforge.net;
	Mon, 15 Dec 2014 21:54:47 +0000
Received: by mail-ig0-f179.google.com with SMTP id r2so5921687igi.6
	for <bitcoin-development@lists.sourceforge.net>;
	Mon, 15 Dec 2014 13:54:40 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=1e100.net; s=20130820;
	h=x-gm-message-state:mime-version:in-reply-to:references:from:date
	:message-id:subject:to:cc:content-type;
	bh=ezoQeS8uLwCjzA2Csb7Nl0f7LMouny9m+WwYKKILkso=;
	b=lwc48ljVnUNBIkEjp9UILRFwZ70mNYbyArklXB0s6vS0SBXbgBUgh6469sGxO8SLhK
	g3Fac9Br2j4+/GM5hkHVrdmNmj3DOX0Uy96BzCYHM6uH2uXo1DgqaH+WD2takObrxwnu
	Cv8Cu4PQD47ilEjw9W6WjzBFvpM1VoeinlU1RmgeiLl5ibS9UI5rjnp6LTc/19YqSlQr
	poH56zALNKnZYZ5T63ZoJQzTetwNPbCWlR+zVeYN9VCxIAvWZA2bxDjAHd1RXoCjXxDD
	pNCBb5sHD11cmOOc3yd9Qymv+G17ZBEDluzaVipkcCcX3lz21FUXH8EvcACGWtGkzPFu
	D4hg==
X-Gm-Message-State: ALoCoQkD9gHoTyG3zAHhmgwvhvB+VQPK1XNRzZDvmFlJtFBAgLP2k9Yz+bGvHG2pxAngakY5k+ij
X-Received: by 10.50.43.136 with SMTP id w8mr19848221igl.33.1418680480614;
	Mon, 15 Dec 2014 13:54:40 -0800 (PST)
MIME-Version: 1.0
Received: by 10.107.135.76 with HTTP; Mon, 15 Dec 2014 13:54:20 -0800 (PST)
In-Reply-To: <CAApLimjMkPvi+JfXMWsgS+tFCxXLivZc=eptuLn7z72O6CE6SA@mail.gmail.com>
References: <20141215124730.GA8321@savin.petertodd.org>
	<CADJgMztRdNWyogPCjv64qpUHcvGDiOZDVAkv5Ra8hn25Z9xa8w@mail.gmail.com>
	<CAJHLa0OvDPazCQVonhokm0KopN-WYj0_PP_LO8gPewc1LG5Qow@mail.gmail.com>
	<CAApLimi+Jskwn=70+cfqr8=d55CKJFpBDBmW48zJk24PSBZczg@mail.gmail.com>
	<CAJHLa0MDftgnxB5itQS2jKqZsPj2vg-=SM+jVqUDxKGRZjuukA@mail.gmail.com>
	<CAApLimjMkPvi+JfXMWsgS+tFCxXLivZc=eptuLn7z72O6CE6SA@mail.gmail.com>
From: Jeff Garzik <jgarzik@bitpay.com>
Date: Mon, 15 Dec 2014 16:54:20 -0500
Message-ID: <CAJHLa0OcSNQTDmseUyu7jrb4HyMQy19emEER8rkjT=cexmAENQ@mail.gmail.com>
To: Cory Fields <lists@coryfields.com>
Content-Type: multipart/alternative; boundary=089e011602a8191ea4050a484adb
X-Spam-Score: -0.6 (/)
X-Spam-Report: Spam Filtering performed by mx.sourceforge.net.
	See http://spamassassin.org/tag/ for more details.
	-1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for
	sender-domain
	-0.0 SPF_PASS               SPF: sender matches SPF record
	1.0 HTML_MESSAGE           BODY: HTML included in message
	-0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from
	author's domain
	0.1 DKIM_SIGNED            Message has a DKIM or DK signature,
	not necessarily valid
	-0.1 DKIM_VALID Message has at least one valid DKIM or DK signature
X-Headers-End: 1Y0dbK-0007D4-Iw
Cc: Bitcoin Dev <bitcoin-development@lists.sourceforge.net>
Subject: Re: [Bitcoin-development] Recent EvalScript() changes mean
 CHECKLOCKTIMEVERIFY can't be merged
X-BeenThere: bitcoin-development@lists.sourceforge.net
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: <bitcoin-development.lists.sourceforge.net>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
	<mailto:bitcoin-development-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=bitcoin-development>
List-Post: <mailto:bitcoin-development@lists.sourceforge.net>
List-Help: <mailto:bitcoin-development-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
	<mailto:bitcoin-development-request@lists.sourceforge.net?subject=subscribe>
X-List-Received-Date: Mon, 15 Dec 2014 21:54:48 -0000

--089e011602a8191ea4050a484adb
Content-Type: text/plain; charset=UTF-8

If code movement is not compressed into a tight time window, code movement
becomes a constant stream during development.  A constant stream of code
movement is a constant stream of patch breakage, for any patch or project
not yet in-tree.  The result is to increase the work and cost on any
contributor whose patches are not immediately merged.

For the record, since this is trending reddit, I __do__ support the end
result of 0.10 refactoring, the work towards the consensus lib.

My criticism is of a merge flow which _unintentionally_ rewards only
certain types of patches, and creates disincentives for working on other
types of patches.







On Mon, Dec 15, 2014 at 4:19 PM, Cory Fields <lists@coryfields.com> wrote:
>
> On Mon, Dec 15, 2014 at 2:35 PM, Jeff Garzik <jgarzik@bitpay.com> wrote:
> > On Mon, Dec 15, 2014 at 1:42 PM, Cory Fields <lists@coryfields.com>
> wrote:
> >>
> >> That's exactly what happened during the modularization process, with
> >> the exception that the code movement and refactors happened in
> >> parallel rather than in series. But they _were_ done in separate
> >> logical chunks for the sake of easier review.
> >
> >
> > "That's exactly what was done except it wasn't"
> >
> > Yes, in micro, at the pull request level, this happened
> > * Code movement
> > * Refactor
> >
> > At a macro level, that cycle was repeated many times, leading to the
> > opposite end result:  a lot of tiny movement/refactor/movement/refactor
> > producing the review and patch annoyances described.
> >
> > It produces a blizzard of new files and new data structures, breaking a
> > bunch of out-of-tree patches, complicating review quite a bit.  If the
> vast
> > majority of code movement is up front, followed by algebraic
> > simplifications, followed by data structure work, further patches are
> easy
> > to review/apply with less impact on unrelated code.
> >
>
> I won't argue that at all because it's perfectly logical, but in
> practice that doesn't translate from the macro level to the micro
> level very well. At the micro level, minor code changes are almost
> always needed to accommodate useful code movement. Even if they're not
> required, it's often hard to justify code movement for the sake of
> code movement with the promise that it will be useful later.
>
> Rather than arguing hypotheticals, let's use a real example:
> https://github.com/bitcoin/bitcoin/pull/5118 . That one's pretty
> simple. The point of the PR was to unchain our openssl wrapper so that
> key operations could be performed by the consensus lib without
> dragging in bitcoind's structures. The first commit severs the
> dependencies. The second commit does the code movement from the
> now-freed wrapper.
>
> I'm having a hard time coming up with a workflow that would handle
> these two changes as _separate_ events, while making review easier.
> Note that I'm not attempting to argue with you here, rather I'm
> genuinely curious as to how you'd rather see this specific example
> (which is representative of most of my other code movement for the
> libbitcoinconsensus work, i believe) handled.
>
> Using your model above, I suppose we'd do the code movement first with
> the dependencies still intact as a pull request. At some later date,
> we'd sever the dependencies in the new files. I suppose you'd also
> prefer that I group a bunch of code-movement changes together into a
> single PR which needs little scrutiny, only verification that it's
> move-only. Once the code-movement PRs are merged, I can begin the
> cleanups which actually fix something.
>
> In practice, though, that'd be a massive headache for different
> reasons. Lots in flux with seemingly no benefits until some later
> date. My PR's can't depend on eachother because they don't actually
> fix issues in a linear fashion. That means that other devs can't
> depend on my PRs either for the same reason. And what have we gained?
>
> Do you find that assessment unreasonable?
>
> Cory
>


-- 
Jeff Garzik
Bitcoin core developer and open source evangelist
BitPay, Inc.      https://bitpay.com/

--089e011602a8191ea4050a484adb
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div><div>If code movement is not compressed into a tight =
time window, code movement becomes a constant stream during development.=C2=
=A0 A constant stream of code movement is a constant stream of patch breaka=
ge, for any patch or project not yet in-tree.=C2=A0 The result is to increa=
se the work and cost on any contributor whose patches are not immediately m=
erged.<br><br></div>For the record, since this is trending reddit, I __do__=
 support the end result of 0.10 refactoring, the work towards the consensus=
 lib.<br><br></div>My criticism is of a merge flow which _unintentionally_ =
rewards only certain types of patches, and creates disincentives for workin=
g on other types of patches.<br><br><br><div><div><br><br><br><div><br></di=
v></div></div></div><div class=3D"gmail_extra"><br><div class=3D"gmail_quot=
e">On Mon, Dec 15, 2014 at 4:19 PM, Cory Fields <span dir=3D"ltr">&lt;<a hr=
ef=3D"mailto:lists@coryfields.com" target=3D"_blank">lists@coryfields.com</=
a>&gt;</span> wrote:<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0=
 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=3D"">On Mon,=
 Dec 15, 2014 at 2:35 PM, Jeff Garzik &lt;<a href=3D"mailto:jgarzik@bitpay.=
com">jgarzik@bitpay.com</a>&gt; wrote:<br>
&gt; On Mon, Dec 15, 2014 at 1:42 PM, Cory Fields &lt;<a href=3D"mailto:lis=
ts@coryfields.com">lists@coryfields.com</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; That&#39;s exactly what happened during the modularization process=
, with<br>
&gt;&gt; the exception that the code movement and refactors happened in<br>
&gt;&gt; parallel rather than in series. But they _were_ done in separate<b=
r>
&gt;&gt; logical chunks for the sake of easier review.<br>
&gt;<br>
&gt;<br>
&gt; &quot;That&#39;s exactly what was done except it wasn&#39;t&quot;<br>
&gt;<br>
&gt; Yes, in micro, at the pull request level, this happened<br>
&gt; * Code movement<br>
&gt; * Refactor<br>
&gt;<br>
&gt; At a macro level, that cycle was repeated many times, leading to the<b=
r>
&gt; opposite end result:=C2=A0 a lot of tiny movement/refactor/movement/re=
factor<br>
&gt; producing the review and patch annoyances described.<br>
&gt;<br>
&gt; It produces a blizzard of new files and new data structures, breaking =
a<br>
&gt; bunch of out-of-tree patches, complicating review quite a bit.=C2=A0 I=
f the vast<br>
&gt; majority of code movement is up front, followed by algebraic<br>
&gt; simplifications, followed by data structure work, further patches are =
easy<br>
&gt; to review/apply with less impact on unrelated code.<br>
&gt;<br>
<br>
</span>I won&#39;t argue that at all because it&#39;s perfectly logical, bu=
t in<br>
practice that doesn&#39;t translate from the macro level to the micro<br>
level very well. At the micro level, minor code changes are almost<br>
always needed to accommodate useful code movement. Even if they&#39;re not<=
br>
required, it&#39;s often hard to justify code movement for the sake of<br>
code movement with the promise that it will be useful later.<br>
<br>
Rather than arguing hypotheticals, let&#39;s use a real example:<br>
<a href=3D"https://github.com/bitcoin/bitcoin/pull/5118" target=3D"_blank">=
https://github.com/bitcoin/bitcoin/pull/5118</a> . That one&#39;s pretty<br=
>
simple. The point of the PR was to unchain our openssl wrapper so that<br>
key operations could be performed by the consensus lib without<br>
dragging in bitcoind&#39;s structures. The first commit severs the<br>
dependencies. The second commit does the code movement from the<br>
now-freed wrapper.<br>
<br>
I&#39;m having a hard time coming up with a workflow that would handle<br>
these two changes as _separate_ events, while making review easier.<br>
Note that I&#39;m not attempting to argue with you here, rather I&#39;m<br>
genuinely curious as to how you&#39;d rather see this specific example<br>
(which is representative of most of my other code movement for the<br>
libbitcoinconsensus work, i believe) handled.<br>
<br>
Using your model above, I suppose we&#39;d do the code movement first with<=
br>
the dependencies still intact as a pull request. At some later date,<br>
we&#39;d sever the dependencies in the new files. I suppose you&#39;d also<=
br>
prefer that I group a bunch of code-movement changes together into a<br>
single PR which needs little scrutiny, only verification that it&#39;s<br>
move-only. Once the code-movement PRs are merged, I can begin the<br>
cleanups which actually fix something.<br>
<br>
In practice, though, that&#39;d be a massive headache for different<br>
reasons. Lots in flux with seemingly no benefits until some later<br>
date. My PR&#39;s can&#39;t depend on eachother because they don&#39;t actu=
ally<br>
fix issues in a linear fashion. That means that other devs can&#39;t<br>
depend on my PRs either for the same reason. And what have we gained?<br>
<br>
Do you find that assessment unreasonable?<br>
<span class=3D"HOEnZb"><font color=3D"#888888"><br>
Cory<br>
</font></span></blockquote></div><br clear=3D"all"><br>-- <br><div class=3D=
"gmail_signature">Jeff Garzik<br>Bitcoin core developer and open source eva=
ngelist<br>BitPay, Inc. =C2=A0 =C2=A0 =C2=A0<a href=3D"https://bitpay.com/"=
 target=3D"_blank">https://bitpay.com/</a></div>
</div>

--089e011602a8191ea4050a484adb--