Re: [PATCH v2 0/1] scalar: move to the top-level, test, CI and "install" support

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 24, 2022, 11:59 UTC
Message-ID: <220624.86y1xmi5wo.gmgdl@evledraar.gmail.com>
In-Reply-To: <c93c8e75-6c88-ac99-d8c3-1e2e7dd06ea3@github.com>

On Thu, Jun 23 2022, Derrick Stolee wrote:

Show 28 quoted lines

> On 6/23/2022 11:30 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Thu, Jun 23 2022, Derrick Stolee wrote:
>> 
>>> On 6/23/2022 6:26 AM, Ævar Arnfjörð Bjarmason wrote:
>>>> This one-patch series integrates the "scalar" command to the
>>>> top-level, meaning we build the "scalar" binary by default, and run
>>>> its tests on "make test" and in CI. We'll also build and test its
>>>> documentation. We now also have "install" support, both for the
>>>> program and its docs, but you'll need to:
>>>>
>>>>     make <install-target> INSTALL_SCALAR=Y
>>>>
>>>> I'm sending this out now to avoid needless duplicate work.
>>>
>>> As mentioned on the list earlier, Victoria is taking over the
>>> remaining work to complete the Scalar project. Nothing has been
>>> sent to the list because we didn't want to cause a distraction
>>> from the release window.
>> 
>> I was on the fence about sending this out, but given that the "CI"
>> thread was going on until the start of that window, and wanting to save
>> her the work of re-discovering the subtle issues with the integration
>> I'd already fixed I thought it was better ot send it out.
>
> The CI thread was halted not just because of the release window,
> but because of a change in who was doing the work and taking time
> to revisit the plan of record. This was communicated on-list [1].

Part of this is me reading between the lines, but as mentioned in the CL I thought it had more to do with the bug I pointed out on the CI series's 2/2.

Show 15 quoted lines

> [1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.2206132246010.353@tvgsbejvaqbjf.bet/
>
> The point is that the end goal is still "Get Scalar out of contrib/"
> but the questions that are still being worked out are things like:
>
> 1. _When_ do we integrate Scalar into CI?
> 2. _When_ do we move Scalar out of contrib/?
> 3. _When_ do we implement the remaining Scalar features?
>
> All three of these questions depend on what the final result will
> look like, and Victoria is still working on developing her own
> opinion on that before presenting it to the list in a complete
> form.
>
> Your patch here is interrupting that process.

Coordination & collaboration can be difficult, but generally speaking we try to use the ML as a means to decide how topics move forward, and for coordination.

In this case I understand that there was off-list discussion about how a part of the codebase should be moving forward among a group of co-workers who contribute to git.

I don't think there's anything wrong with that, but this really seems like an overly hostile response to someone who wasn't part of that process.

Especially as I'm not insisting that I be the one to drive anything forward on the scalar topic, I think it makes much more sense that it's Victoria.

This patch is just offered as a way to help that effort along. Perhaps she'd ack it as-is, find it useful as it reveals some edge cases she didn't know about, or drop it and go for some other approach. Whatever gets us to the end-goal sooner than later is fine by me.

Once you "git rm" the scalar Makefile there's not really a lot of ways you can go other than something approximating the upthread patch. Given that some of the edge cases are tricky I deemed it worthwhile to share it.

Show 23 quoted lines

>>> Victoria is taking time to incorporate your previous thoughts on
>>> how Scalar is built and its location in the codebase and create
>>> a complete narrative of how to get from our current state to that
>>> point.
>> 
>> I wasn't sure she was even aware of it, and given that the WIP patch I
>> saw in my "git fetch" was pretty much a subset of the upthread v1 it
>> seemed that there was needless duplicate work going on.
>> 
>> It seemed clear that that WIP patch was attempting to head in the same
>> direction, but hadn't yet discovered some of the hurdles with
>> e.g. documentation building & installation that I'd fixed
>> already. There's also the CMake integration, which I finished up for
>> this v2.
>
> Poking around in people's forks to discover what they have as WIP
> is not a good measurement of what work is being done or not. A lot
> of mental energy is being spent in this area.
>
> Saying "What I see in this person's fork isn't good enough" is not
> a reason to move forward on your own. WIP work is WORK IN PROGRESS
> and is not assumed to be complete or up to expectation for review
> on the list.

I'm not saying that, but "you seem to be trying to do X, and may or may not be aware of a past patch that does X, here is is in case it help!.

Show 6 quoted lines

>> FWIW I have this local change queued on top of this v2, it's all
>> cosmetic, but probably a good idea.
>
> Please stop motivating current patches by work that you have been
> playing with in your local tree. You do this frequently but it is
> not helpful.

After I submit patches to the ML I tend to "bump" the branch I sent it from, that range-diff was against the submitted v2 to local changes I applied afterwards, having missed some spots in the initial submission.

I'm not clear on what part of this you're taking issue with...

Show 5 quoted lines

>> The $(SCALAR_SOURCES) bit is something I missed, but which Victoria
>> didn't in her WIP patch (I stole it from there).
>
> You also mention this in your cover letter (but is notably absent from
> the patch itself).

...yes, hence the "local change queued on top" above, it's a minor detail I noticed after submitting the patch.

> I can only attribute your insistence here as a lack of respect for
> your fellow contributors. [...]

I really think that's unhelpful. Let's try to assume some mutual good intent. I think I've explained my motivations above.

> [...]There is no rush for this to happen immediately, so the only
> reason is that you don't trust that it would be done correctly without
> you submitting the work.

Some of the edge cases in the Makefile integration are subtle. I trust that someone looking at it would probably discover those eventually. I think it took me about a day of poking around, much of which was a slow ping-pong with the CI. So someone with a local Windows box could do it faster.

But if I'd have gotten a patch from my future self with all the learned edge cases beforehand I'd have appreciated, so I figured I'd send it in.