[bug#57598] [PATCH] doc: Update contribution guidelines on patches, etc.

[Liliana Marie Prikler]

Am Donnerstag, dem 08.09.2022 um 13:12 +0200 schrieb Maxime Devos:
> On 07-09-2022 10:09, Liliana Marie Prikler wrote:
> > Am Dienstag, dem 06.09.2022 um 22:21 +0200 schrieb Maxime Devos:
> > > It is, to my knowledge, not forbidden to mention non-free
> > > software by name in code, as long as its not a recommendation
> > > (explicit or implied).
> > Indeed, there is no hard rule, hence "avoid" rather than "forbid".
> What I also meant is, that to my knowledge there is no soft rule
> either.  Again, why should we avoid this, what's the point of that?
In descriptions, it is wise to do so because it helps software stand on
its own merits rather than just "being a clone of this thing you aren't
allowed to have" (this is real criticism pointed at us from the
proprietary software embracers).  See for instance minetest, whichisy

> How does ignoring a test fix the technical issue identified by the
> test (sometimes, the technical issue being a bug in the test itself)?
It fixes the technical issue that an otherwise functional package
(w.r.t. the N tests that don't fail) builds.  This is a particularly
useful distinction with tests that require a network connection and
thus have to fail in a build container, or are known flaky upstream and
thus cause reproducibility issues.

> > There still is the difference that phases are clearly delimited
> > while snippets are a block of code that shouldn't get too large.
> Snippets are delimited clearly as well, though, with the 'snippet'
> field?
> And the limitations of snippet length and phases length are the same
>  -- no limits, though conciseness is appreciate as always. 
True, but phases can be made to do just one thing, whereas snippets
have to fix everything that's wrong in more or less one (begin ...). 
This is a noticable distinction.

> 
> > I think my version at least hinted at this practice in a more
> > concise way, so it's not impossible to mention. [...]
> I agree it's possible -- as I replied previously:
> > I suppose a section could be added somewhere to properly document
> > the 'embedding store file names' practice, and insert a
> > cross-reference,
> I don't think documenting the how of the practice should be done
> in this section, properly explaining 'search-input-file' / 'search-
> input-directory', 'inputs / native-inputs', 'bash' being an implicit
> input but you still have to add it to 'inputs' in some cases because
> of cross-compilation, this-package-input and this-package-native-
> input ... would make the
> subsubsection a bit too long I think, distracting from other
> situations, hence the proposal for a cross-reference.
> How about leaving the 'how to embed store file names' for a separate
>  documentation patch and section, adding a cross-reference later?
See above, "I suppose a section could be added..." means I'm somewhat
indifferent to whether it's done now or later; I would however very
much prefer a wording that points people toward this practice existing,
even if they have to go look in the code for examples.  Alternatively,
a footnote for the most common case (search-input-file inputs
"bin/command") ought to suffice for the time being.

> 
> 
> > I think calling back to a guiding principle in and of itself shows
> > that the section has grown too long to remember it by the point you
> > come to this example,
> This has nothing to do with length and remembering, but rather with
> explaining why a phase must be used -- to explain that, I state which
> principle applies (as mentioned previously). If I removed the
> explanations, I would just be stating how to do things, without
> giving a logical reasoning on the 'why'.
IMHO, I think a reader who remembers the guiding principles should see
that it applies.

> > and I think that's more problematic than merely the
> > callback. If you didn't need to divide this into subsubsections,
> > you could introduce the guiding principles in a way that feels more
> > natural.
> I consider it more natural to have the 'guiding principles' _before_
> the concrete cases, as they are meant to be 'guiding' and
> 'principles'. 
> It's like 'starting from first principles', there introducing the
> first principles as you go is ad-hoc.
> The guiding principles also need to be outside the examples, in case
>  one of the examples doesn't apply to the packager's use case, such
>  that they can fall-back to the guiding principles.
> Also, in your patch you are dividing things in subsubsections as
> well, just under a different name and different representation (table
> entries in a subsection), as mentioned previously.
A table entry is not a subsection, as much as you want it to be that.

Also, your guiding principles are (with one exception) really just
invariants that ought to hold for the source field of a package.  
As such, I think it would be easier to state "A package's source should
be the smallest corresponding source in terms of uncompressed file
size.  This corresponding source must consist only of free software
(note Free Software) and should build on all platforms supported by
upstream."  Note how smallest naturally implies unbundling bundled
sources.

> > I still find this wording very confusing. Perhaps "To add new
> > functionality, a patch is almost always the best choice. For one,
> > it is likely that the new functionality requires changing multiple
> > lines of source code, which is more convenient to do with a patch
> > than with a snippet. Further, patches can be taken from and
> > submitted to upstreams more easily. If your patch has not been
> > submitted to upstream, consider doing so."
> It loses some information (that patches are preferred) and
> (after re-adding the conclusion) is more verbose, which appears
> to be considered very important.
Which conclusion is there to re-add?  The conclusion is already stated
in the beginning: patches are almost always the best choice.  Then two
reasons as for why.  The part w.r.t. upstreaming changes has also been
addressed.

> > An enumeration ought to have at least three elements (otherwise
> > it's just a pair), and I think if we do proper counting and omit
> > no-brainers, such as the "only free software" part that has already
> > been mentioned, we come very close to skirting that line.
> The "only free software" is mentioned elsewhere, yes, but it is one
> of the principles for deciding between snippets, phases and patches.
> While you call it a no-brainer, it is sometimes neglected, so it
> sounds important to me to explicitly list it. 
> Merging the 3th and 4th @item, I count 4 principles, so it fits with
> an enumeration.
> Also, I'm not following your point here -- your complaint was that
> they aren't guiding principles (based on the number of them), but
> your response is that they might not form an enumeration?  They are
> named the guiding principles, not the guiding enumeration.  What have
> enumerations to do with anything?
I'm using enumeration as a super term here, which can be
((sub)sub)sections, chapters, list elements, whatever, and my claim is
that we barely have enough principles to allow the use of a plural. 
Adding to that, now that I think of it, I also doubt their usefulness
in guiding.  "Use whatever feels convenient, but note that that might
be subjective" is more useful at the end of the section when a user
didn't find what they were looking for than at the start.

> > > > which (along with its sheer length) is my main
> > > > complaint with the way you've phrased things.
> > > (I'm assuming "its = the patch as a whole" here)
> > > 
> > > I could remove another section of the manual to compensate for
> > > the additional length, but I doubt that's what you intended.  I
> > > do not see the problem with the sheer length -- we have a bit of
> > > a documentation problem in Guix, there is lots of useful
> > > information that is currently undocumented.
> > > I do not think there have been any complaints about the manual
> > > being too long, if anything, it's too short.
> > I personally tend towards "less verbose", hence my complaint of
> > describing something with many words that could be described with
> > fewer. A section can still be too long while the chapter around it
> > is too short.
> Do you have anything in particular in mind?
This is more of a broad statement that applies to the patch as a whole
than to any of its constituent parts in particular.  However, in some
cases where I think it's particularly noticable, I'll try to point out
shorter formulations.

> > > I've written some documentation, it was originally a bit hard to
> > > follow so in a next version I've restructured it a bit and
> > > explained more, this restructuring and explanation entailed some
> > > additional length.
> > > 
> > > There had been some proposals for additional cases to document,
> > > so they were added, increasing the length.  You have added new
> > > information is your patch, it was considered useful so I've
> > > integrated some of it in my patch, increasing the length.  (I
> > > didn't integrate all of the new parts, if I did, it would
> > > increase even further.  (If desired, in can integrate the rest,
> > > at cost of some time.)).
> > My patch did not just state some things you missed, it also omitted
> > things that I think are either not necessary or probably better
> > documented elsewhere.
> What particular things do you have in mind, and where do you
> think they should better be documented?  I can move things
> around a bit and add cross-references. 
> > 
> > > I do not see what the problem is with additional length as long
> > > as this additional length comes with additional useful
> > > information and the manual is well-structured (e.g. with
> > > (sub)(sub)sections, chapters and indices) -- we do not have a
> > > page limit.
> > > 
> > > At worst, perhaps the same information could perhaps be encoded
> > > with fewer words? I could compare the two patches to see which
> > > one formulates certain information in the fewest words, and
> > > choose the least verbose of the two for each piece of information
> > > that is present in both?
> > > 
> > > Also, comparing the two patches, my patch has 40 more lines, but
> > > about 25 of them are for noting the guiding principles (which are
> > > absent in your patch).
> > > Compensating for that, the patches are about the same length, so
> > > I do not think that 'sheer length' is accurate here.
> > 25 lines calling back to earlier information are, imho, an
> > indicator that the section is too long. Imagine you'd have twenty-
> > five function calls to guiding_principles(n) in your program – at
> > some point, you'd try to cache those.
> (define cached-guiding-principles
>   (delay (list (guiding-principle-0)
>                     [...]
>                     (guiding-principle-24)))) 
> Caching the guiding principles does not reduce the length.
> I don't see the problem with calling back to earlier information.
> Also, it isn't earlier information, there is no nice list of guiding
> principles anywhere else.
At the risk of responding jokingly to what was meant serious: I didn't
know we suddenly gained 20 guiding principles.

> 
> > > > Going down to subsubsections just to find out where patches are
> > > > appropriate, is imho overkill.
> > > The 'going down to subsubsection' is the case for your patch too,
> > > though? In my case, it's a subsubsection, in your case it's a
> > > table
> > > entry inside a subsection, both are the same level of nesting.
> > These are still two very different kinds of nesting. A table fits
> > onto a (virtual) page more easily than several subsections.
> I suppose table items might take two less line or so less than
> subsubsections, but I don't think that's sufficient reason to step
> away from a nice section structure.
Another reason is that you can end a table in the middle of a section,
which you can't do with subsections.  Hence why I'm able to put a
remark at the bottom, which you have to clumsily fit into the top.

> > > Also, it's a matter of different structure -- in my v2 and v3
> > > patch, I have a 'problem -> solution' structure -- the idea is
> > > that the packages has a problem, they look at the section, they
> > > read the subsubsection names, select the
> > > subsubsection that matches their problem and read the solution --
> > > in short, the idea is to provide a solution to the problem.
> > > 
> > > Your structure is the other way around -- for solutions (patches,
> > > snippets, phases), it gives the permitted problems to apply it
> > > to.
> > > 
> > > So yes, your patch is more convenient for finding out where
> > > patches are appropriate.  I do not see the benefit of that though
> > > -- a new contributor packaging a thing wouldn't know in advance
> > > which solutions could be appropriate for them (your 'solution ->
> > > problem' patch?), rather, they start with a problem and are
> > > searching for an appropriate solution (my problem->solution
> > > patch).
> > I think this idea can be debunked pretty easily. If I give you a
> > hammer and I tell you "this is a hammer, you can use it to put
> > nails into a wall", and you have a nail and you want to put it into
> > a wall, you won't go "oh no, however will I put this nail into a
> > wall?" – you will simply use the hammer to do so.
> The patch does this, currently.  It already proposes a number of
> hammers (patches, snippets and phases) and purposes (adding new
> functionality, fixing technical issues, unbundling, ...). 
> Also, the scenario "oh no, however will I put this nail into a wall"
> actually happened -- see the Shepherd discussion, where there was
> a lot of disagreement on how nails (= small work-around in the
> Makefile) should be put in walls (= patches, snippet, phase?).   It
> was the whole reason to start writing a documentation patch.
You might want to add a link here if it supports your argument, but
without looking at the discussion this rather sounds like "oh no, I
have three hammers, which one do I pick?" – which, fair enough, is
still a problem, but one that neither of our patches would cause imho.

> > Of course, for this to work I also have to tell you *how* to use a
> > hammer to put nails into a wall, but that's exactly what I did in
> > my patch by inserting the right notes into the Guix manual.
> Also already the case. 
> > My solution->problem approach has the benefit, that folks can just
> > go over all the solutions, check if their problem fits, and apply
> > the one that says "here, use this".
> A problem->solution structure is useful for that too?
> And it already lists all the solutions (snippets, phases and patches)
> and information to decide whether the solution fits their problem
> (the guiding principles, and the worked-out cases).
Again, I believe you're overselling the guiding principles.

> > And if they don't find anything, they see the handy little line at
> > the bottom saying "use whatever you think is convenient".
> Nowhere did the patch imply that the listed cases were all cases. In
> fact, in two places in the introduction it is implied that the
> examples are not exhaustive, and that they can choose according to
> convenience  [...]
Emphasis on handy little line rather than needing to be told twice
(particularly if people have no idea what to expect due to not having
looked at the worked-out cases yet).

> >  I also expand a little on the benefits and drawbacks of
> > these approaches as you would when describing design patterns.
> This is also done in my patch. [...]
This is not about the contained information, but the structure of the
contained information.

My solution->problem style follows roughly this style:
1. solution
2. problems it is known to solve
3. how to use
4. properties, caveats, etc.

Your problem->solution style roughly has the following:
1. problem
2. (set of) solution(s)
3. if more than one solution, maybe a help to select

This makes it so that people might have to go to a different subsection
than the one they read for their solution to find out about potential
caveats, e.g. not embedding store paths in a snippet.

> > Your problem->solution approach instead leaves people wondering
> > when their particular use case has not been described.
> See my reply to ‘And if they don't find anything, they see the handy
> little line at the bottom saying "use whatever you think is
> convenient’. 
> > It gives them a solution rather than the tools to build solutions
> > with.
> It does give the tools: snippets, patches and phases.
As far as I read, it describes none of those.  It puts out guiding
principles and some already worked-out cases.

> Also, "giving the tools to build solutions with" does not help with
> the problem that I aimed to solve -- there was disagreement on what
> the appropriate tools are (see: Shepherd), so it not just needs to
> give the tools, but also the solutions.
I don't see how my patch lacks this information however.  In
particular, for review purposes, mine should be easier to work with. 
For instance, the reviewer sees a hash embedded in a snippet, sees in
the snippet entry that you shouldn't do that, and can thus say "nope,
don't do a snippet here".

Regardless of which patch we pick, it should not mandate a particular
solution in potentially contentious cases, and also point towards the
right solution.  See our discussions on the individual solutions on
points in which I believe you've errored.

Cheers