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: Katherine Cox-Buday
Subject: [bug#34959] Acknowledgement ([PATCH] Add Multiple Common Lisp Packages)
Date: Thu, 28 Mar 2019 12:38:55 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Ricardo Wurmus <address@hidden> 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 via email to

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