OpenPGP_0x49E3EE22191725EE.ascDescription: OpenPGP public key
|
| From: | Maxime Devos |
| Subject: | [bug#57598] [PATCH] doc: Update contribution guidelines on patches, etc. |
| Date: | Thu, 8 Sep 2022 13:12:43 +0200 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 |
On 07-09-2022 10:09, Liliana Marie Prikler wrote:
Am Dienstag, dem 06.09.2022 um 22:21 +0200 schrieb Maxime Devos:We also avoid spelling out the non-free filename where possible, preferring keep lists over remove lists, which this kind of patches would be.Should we? I'm not seeing the point of that. I have not experienced any such avoidance myself, see e.g. 'tennix', 'neverball' and 'shogun'. 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?
How does ignoring a test fix the technical issue identified by the test+@subsubsection Fixing technical issues (compilation errors, test failures, other bugs ...) [...]I am pretty sure that most of these are *not* done in snippets, but rather phases, if they only affect Guix. In particular, grep for failing-tests and you will find a few phases disabling them.I do not think that ignoring a test counts as a bug fix. I'll add it to this subsubsection, at cost of some additional length.I do think it counts as "fixing technical issues such as test failures".
In fact, as far as files that will not be installed are concerned, I think phases ought to be preferred, because they're easier to take away if an actual fix is made.I do not see a difference in hardness/easyness between removing a phase and removing a snippet (both are just a matter of opening an editor, pointing it at gnu/packages/... and removing a few lines), though I do consider removing a patch to be slightly harder (because gnu/local.mk is easy to forget).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.
For the store path embedding, that's a rather roundabout way of saying that contributers *ought to* embed store paths of certaing things, such as commands invoked via exec et al.It's not? It's kind of implied, yes, but the purpose isn't being a 'you should embed store paths' (subsub)section, but rather, 'if you go embedding store paths (at least for fixing a technical issue), do it in a phase'. I'm not following what the complaint is, I suppose a section could be added somewhere to properly document the 'embedding store file names' practice, and insert a cross-reference, but that wasn't the purpose of the patch and going by later responses, you seem opposed to making things longer. The alternative would be to remove this information, but then valuable information would be lost (there had been some cases where store file names were embedded in origin).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 don't think documenting the how of the practice should be doneI suppose a section could be added somewhere to properly document the 'embedding store file names' practice, and insert 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?
This has nothing to do with length and remembering, but rather withOtherwise, if the store +file name were embedded in the source, the result of @command{guix build +--source} would be unusable on non-Guix systems and also likely unusable +on Guix systems of another architecture.Why are you repeating a guiding principle?I'm showing why, in this case, a phase must be used, by noting that not doing so would be contrary to one of the principles. If not repeating the principle is desired, I could perhaps number them, and refer to the principles by number instead of restating them? Would reduce the length a little.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,
I consider it more natural to have the 'guiding principles' _before_ theand 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.
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.
It loses some information (that patches are preferred) and+@subsubsection Adding new functionality +To add new functionality, a patch is almost always the most convenient +choice of the three -- patches are usually multi-line changes, which are +convenient to do with patches and inconvenient to do with phases or +snippets.Uhm, what? Patches are the preferred form of patches?No, I meant that patches are (usually) the preferred method for adding new functionality, and that multi-line changes are convenient to do with patches. ‘which’ refers to the ‘multi-line changes’ here, not ‘patches’.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."
The "only free software" is mentioned elsewhere, yes, but it is one[...] Overall, I'm not convinced that we have enough guiding principles to call them that,I don't think there's any lower limit on how many guiding principles to have, except for perhaps 2 (because otherwise it should have been singular or there aren't any). At how few guiding principles stop the guiding principles from being guiding principles for you, and why? For example, on <https://www.gnu.org/philosophy/free-sw.html>, four guiding principles are mentioned (which are named 'essential freedoms' there), and <https://en.wikipedia.org/wiki/Guiding_Principles> has 5 ‘Guiding Principles’.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.
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?
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?
What particular things do you have in mind, and where do youI'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.
(define cached-guiding-principlesI 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.
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.
I suppose table items might take two less line or so less thanGoing 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.
The patch does this, currently. It already proposes a number of hammersAlso, 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.
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.
Also already the case.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.
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).
Nowhere did the patch imply that the listed cases were all cases. In fact,And if they don't find anything, they see the handy little line at the bottom saying "use whatever you think is convenient".
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. E.g.,
When using snippets, the bundled library does not occur in the source
returned by @code{guix build --source}, so users and reviewers do not
have to worry about whether the bundled library contains malware,
whether it is non-free, if it contains pre-compiled binaries ... There
are also less licensing concerns: if the bundled libraries are removed,
it becomes less likely that the licensing conditions apply to people
sharing the source returned by @command{guix build --source}, especially if
the bundled library is not actually used on Guix systems.@footnote{This
is @emph{not} a claim that you can simply ignore the licenses of
libraries when they are unbundled and replaced by Guix packages -- there
are less concerns, not none.}
As such, snippets are recommended here.’
See my reply to ‘And if they don't find anything, they see the handy littleYour problem->solution approach instead leaves people wondering when their particular use case has not been described.
It gives them a solution rather than the tools to build solutions with.
It does give the tools: snippets, patches and phases. And as
tools
for deciding between the three for not-yet-documented cases, there
are
the guiding principles. As a demonstration on how to use these
guiding
principles, various cases have been worked out based on the
guiding
principles.
Summarised, it gives both the tools _and_ the solutions.
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.
OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
| [Prev in Thread] | Current Thread | [Next in Thread] |