Re: [PATCH v7 00/17] Upstreaming the Scalar command
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-18 14:33:50
On Wed, Nov 17 2021, Johannes Schindelin via GitGitGadget wrote:
* To avoid mistaking the current patch series for being feature-complete enough to unleash onto end users, I moved the Makefile rules to build HTML/manual pages to a later patch series. [...] * I added two patches that I had planned on keeping in an add-on patch series for later, to build and test Scalar as part of the CI. I am still not 100% certain that it is a good idea to do so already now, but let's see what the reviewers have to say.
I don't expect given [1] that I'll get a reply to this, but as noted there I've had outstanding reviews of this series (including proposed patches) for weeks/months now that haven't been addressed, including the patch-on-top I suggested in the previous round of discussion [2]. Looking at the new README.md it seems that the change of plan between v6..v7 is that after insisting that this command must stay decoupled in contrib/scalar/ and have its own build/test etc. infrastructure, the end-goal now eventual full integration at the top-level. Peeking a branch of yours[3] there's a change[4] in it that's much like my [2], some of the hunks are even the same. I'm quite happy with that end-state, but still don't understand why the "scalar-extra-docs" and "run-scalar-functional-tests" (in your README.md here) couldn't be squashed into this first step, as suggested in my [2]. The end-state is the same, but doing it that way avoids all this path-based churn, setting up build systems that are then going away in the short-to-medium term etc, as well as the new custom just-for-scalar CI integration in this v7, you'd get that and more for free with my [2]. All of that is apparently to:
[...]avoid mistaking the current patch series for being feature-complete enough to unleash onto end users[. ...]
Which is fair enough, but doesn't explain why this back-and-forth churn is needed to reach an end-goal we apparently now almost entirely agree on. I.e. in my [2] it's built & tested by default, but not installed. So end users aren't seeing it. The small bit of proposed in-advanced reference to it in our documentation is there in [2], but that's trivially tweaked, we could defer that too. No end-user would see any of it. So who's being helped by these intermediate steps? As far as I can tell based on your previous comments the perceived necessity these intermediate steps comes from a narrow reading of contrib/README, i.e. that software in this sort of flux *MUST* live in contrib, even if we've got plans & patches in-progress to move it out eventually. Whereas I think we can just document that it's in flux, and skip the intermediate churn to get to the agreed-upon end-state. As before I'm happy to engage with you, but I must say at this point I'm not getting my hopes up much. Cheers. 1. https://lore.kernel.org/git/211110.86czn8hyis.gmgdl@evledraar.gmail.com/ (local) 2. https://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/ (local) 3. https://github.com/dscho/git/tree/vfs-with-scalar 4. https://github.com/dscho/git/commit/f9b8e5d5e7e