guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#34959] Acknowledgement ([PATCH] Add Multiple Common Lisp Packages)


From: Ricardo Wurmus
Subject: [bug#34959] Acknowledgement ([PATCH] Add Multiple Common Lisp Packages)
Date: Thu, 28 Mar 2019 21:43:04 +0100
User-agent: mu4e 1.0; emacs 26.1

Hi Katherine,

>> We usually expect one commit per independent change.  Could you please
>> split up this patch into multiple commits?  You can group package
>> variants like “sbcl-trivial-backtrace” and “cl-trivial-backtrace”, but
>> separate packages generally each should have their own commit.
>
> This will be a non-trivial amount of work, and these will have to be
> chained against each other since this patch forms a dependency graph.
> Given that, can you discuss the benefits of splitting these into
> separate commits? Also is it multiple commits, one patch? Or multiple
> patches?

You can generate one patch per commit with “git format-patch”.  Separate
commits allow us to use the git history to our advantage.  We use git
not only do we track changes but semantic units of change.  We decided
on the unit to be individual packages.

One effect of this is that the summary line of each commit can be exact.
When reading the commit log this keeps surprises to a minimum.

If you are an Emacs user I recommend using the Yasnippet “add” snippet
for quickly generating commit messages for the staged diff.  This makes
it much easier to create the expected commit messages, especially when
there are many commits.  (I use this all the time for R package updates
and additions.)

>>> +(define-public sbcl-trivial-backtrace
>>> +  (let ((commit-hash "ca81c011b86424a381a7563cea3b924f24e6fbeb"))
>>> +    (package
>>> +     (name "sbcl-trivial-backtrace")
>>> + (synopsis "Portable simple API to work with backtraces in Common
>>> Lisp")
>>> + (description "On of the many things that didn't quite get into the
>>> Common Lisp standard was how to get a Lisp to output its call stack
>>> when something has gone wrong. As such, each Lisp has developed its
>>> own notion of what to display, how to display it, and what sort of
>>> arguments can be used to customize it. trivial-backtrace is a simple
>>> solution to generating a backtrace portably.")
>>
>> Please break up long lines like this.  Please run “./pre-inst-env guix
>> lint sbcl-trivial-backtrace”, which will tell you about this problem.
>
> I did run linting, but I wasn't sure how to break up description
> strings. If I just do line-breaks, will that be OK?

Yes.  Line breaks are the right tool here.  In Emacs you can do M-q
while point is on the description string to break up the lines
in the expected fashion.

>>> +     (source
>>> +      (origin
>>> +       (method git-fetch)
>>> +       (uri (git-reference
>>> +             (url "https://github.com/gwkkwg/trivial-backtrace.git";)
>>> +             (commit commit-hash)))
>>
>> Throughout you can use “commit” instead of “commit-hash” — there is no
>> naming conflict.
>
> Yes, I know. I preferred `commit-hash` because that's what it is -- not
> an actual commit. Do we standardize this much, down to variable names?

That’s fine.  I guessed that you used “commit-hash” to avoid a name
clash, so I clarified that this is not necessary.

Personally, I think it’s best to stick with the established conventions
as it makes working on Guix less surprising.

--
Ricardo






reply via email to

[Prev in Thread] Current Thread [Next in Thread]