On Sun, Nov 08, 2015 at 11:59:33AM -0800, Andres Freund wrote: > On November 8, 2015 11:52:05 AM PST, Noah Misch <n...@leadboat.com> wrote: > >On Sun, Nov 08, 2015 at 11:11:42AM -0800, Andres Freund wrote: > >> On November 8, 2015 12:54:07 AM PST, Noah Misch <n...@leadboat.com> wrote: > >> > >> >I have pushed a stack of branches to > >> >https://github.com/nmisch/postgresql.git: > >> > > >> >mxt0-revert - reverts commits 4f627f8 and aa29c1c > >> >mxt1-disk-independent - see below > >> >mxt2-cosmetic - update already-wrong comments and formatting > >> >mxt3-main - replaces commit 4f627f8 > >> >mxt4-rm-legacy - replaces commit aa29c1c > >> > > >> >The plan is to squash each branch into one PostgreSQL commit. In > >> >addition to > >> >examining overall "git diff mxt2-cosmetic mxt3-main", I recommend > >> >reviewing > >> >itemized changes and commit log entries in "git log -p --reverse > >> >--no-merges > >> >mxt2-cosmetic..mxt3-main". In particular, when a change involves > >> >something > >> >you discussed upthread or was otherwise not obvious, I put a > >statement > >> >of > >> >rationale in the commit log. > >> > >> I'm not following along right now - in order to make cleanups the > >plan is to revert a couple commits and then redo them prettyfied? > > > >Yes, essentially. Given the volume of updates, this seemed neater than > >framing those updates as in-tree incremental development. > > I don't like that plan. I don't have a problem doing that in some development > branch somewhere, but I fail to see any benefit doing that in 9.5/master. > It'll just make the history more convoluted for no benefit. > > I'll obviously still review the changes.
Cleanliness of history is precisely why I did it this way. If I had framed the changes as in-tree incremental development, no one "git diff" command would show the truncation rework or a coherent subset. To review the whole, students of this code might resort to a cherry-pick of the repair commits onto aa29c1c. That, too, proves dissatisfying; the history would nowhere carry a finished version of legacy truncation support. A hacker opting to back-patch in the future, as commit 4f627f8 contemplated, would need to dig through this thread for the bits added in mxt3-main and removed in mxt4-rm-legacy. The benefits may become clearer as you continue to review the branches. > Even for review it's nor particularly convenient, because now the entirety of > the large changes essentially needs to be reviewed anew, given they're not > the same. Agreed; I optimized for future readers, and I don't doubt this is less convenient for you and for others already familiar with commits 4f627f8 and aa29c1c. I published branches, not squashed patches, mostly because I think the individual branch commits will facilitate your study of the changes. I admit the cost to you remains high. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers