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 ) 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 ; 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: References: <20141215124730.GA8321@savin.petertodd.org> From: Jeff Garzik Date: Mon, 15 Dec 2014 16:54:20 -0500 Message-ID: To: Cory Fields 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 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 wrote: > > On Mon, Dec 15, 2014 at 2:35 PM, Jeff Garzik wrote: > > On Mon, Dec 15, 2014 at 1:42 PM, Cory Fields > 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
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.

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 workin= g 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:=C2=A0 a lot of tiny movement/refactor/movement/re= factor
> 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.=C2=A0 I= f 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, bu= t 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<= br> 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<= br> 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<= br> 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 actu= ally
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 eva= ngelist
BitPay, Inc. =C2=A0 =C2=A0 =C2=A0https://bitpay.com/
--089e011602a8191ea4050a484adb--