all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
To: Ricardo Wurmus <rekado@elephly.net>
Cc: 34959@debbugs.gnu.org
Subject: [bug#34959] Acknowledgement ([PATCH] Add Multiple Common Lisp Packages)
Date: Thu, 28 Mar 2019 12:38:55 -0500	[thread overview]
Message-ID: <87mule3nm8.fsf@gmail.com> (raw)
In-Reply-To: <87pnqg616m.fsf@elephly.net> (Ricardo Wurmus's message of "Sun, 24 Mar 2019 11:01:37 +0100")

Ricardo Wurmus <rekado@elephly.net> writes:

Hey Ricardo, thanks for the review. I expect to continue work on this
tomorrow, but I wanted to check in on a few things before I get going.

> 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?

>> +(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?

>> +     (home-page "https://common-lisp.net/project/trivial-backtrace/")
>> +     (license license:bsd-style)
>> +     (version (string-append "0.0.0-" (string-take commit-hash 7)))
>
> Please use “git-version” here.  Please also use a revision string, which
> allows us to build version strings that can be sorted, e.g. when the
> base version is unchanged and a newer commit is “smaller” than the
> previous one.
>
>> +     (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?

-- 
Katherine

  reply	other threads:[~2019-03-28 17:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-23 20:14 [bug#34959] [PATCH] Add Multiple Common Lisp Packages Katherine Cox-Buday
     [not found] ` <handler.34959.B.155337211019624.ack@debbugs.gnu.org>
2019-03-23 20:41   ` [bug#34959] Acknowledgement ([PATCH] Add Multiple Common Lisp Packages) Katherine Cox-Buday
2019-03-24 10:01     ` Ricardo Wurmus
2019-03-28 17:38       ` Katherine Cox-Buday [this message]
2019-03-28 20:43         ` Ricardo Wurmus
2019-03-29 17:15           ` Katherine Cox-Buday
2019-03-29 23:19       ` Katherine Cox-Buday
2019-04-07  6:13         ` bug#34959: " 宋文武
2019-04-10 14:46           ` [bug#34959] " Katherine Cox-Buday
2019-04-10 17:00             ` Ricardo Wurmus
2019-04-10 17:57               ` Katherine Cox-Buday
2019-04-10 18:19                 ` Ricardo Wurmus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mule3nm8.fsf@gmail.com \
    --to=cox.katherine.e@gmail.com \
    --cc=34959@debbugs.gnu.org \
    --cc=rekado@elephly.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.