From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:35954) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Z0W-0007bb-9j for guix-patches@gnu.org; Thu, 28 Mar 2019 13:40:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h9Z0V-0001XA-0e for guix-patches@gnu.org; Thu, 28 Mar 2019 13:40:04 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:49299) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h9Z0U-0001Wz-8q for guix-patches@gnu.org; Thu, 28 Mar 2019 13:40:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1h9Z0U-0002DO-4o for guix-patches@gnu.org; Thu, 28 Mar 2019 13:40:02 -0400 Subject: [bug#34959] Acknowledgement ([PATCH] Add Multiple Common Lisp Packages) Resent-Message-ID: From: Katherine Cox-Buday References: <87pnqh4acd.fsf@gmail.com> <87lg15493r.fsf@gmail.com> <87pnqg616m.fsf@elephly.net> Date: Thu, 28 Mar 2019 12:38:55 -0500 In-Reply-To: <87pnqg616m.fsf@elephly.net> (Ricardo Wurmus's message of "Sun, 24 Mar 2019 11:01:37 +0100") Message-ID: <87mule3nm8.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ricardo Wurmus Cc: 34959@debbugs.gnu.org Ricardo Wurmus 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 =E2=80=9Csbcl-trivial-backtrace=E2=80=9D and =E2=80=9Ccl-tr= ivial-backtrace=E2=80=9D, 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 =E2=80=9C./pre-inst-env= guix > lint sbcl-trivial-backtrace=E2=80=9D, which will tell you about this prob= lem. 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 =E2=80=9Cgit-version=E2=80=9D here. Please also use a revisio= n 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 =E2=80=9Csmaller=E2=80=9D= 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 =E2=80=9Ccommit=E2=80=9D instead of =E2=80=9Ccommi= t-hash=E2=80=9D =E2=80=94 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? --=20 Katherine