diff mbox series

[bug#41181] adds emacs-interleave

Message ID -nBsa1KQJSwbENQmZILkCluHwqx_QyjF3PQxKtoNHoOeqeFc1FHy95KjjodciPwOSIH5XOxKIhoHv0Hzb87JVOVy3D1fEb1nD9fDidbeMiU=@protonmail.com
State Accepted
Headers show
Series [bug#41181] adds emacs-interleave | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

guix--- via Guix-patches via May 15, 2020, 8:56 p.m. UTC 
Hello Nicolas,

Thanks for the pointers.  Attached is an updated patch.

Ryan


Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, May 11, 2020 3:10 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> rdes via Guix-patches via guix-patches@gnu.org writes:
>
> > The attached patch adds the emacs-interleave package.
>
> Thank you! Some comments follow.
>
> > Subject: [PATCH] gnu: Add emacs-interleave
>
> Your commit message should conform to Guix rules. Here, you need to add
>
> -   gnu/packages/emacs-xyz.scm (emacs-interleave): New variable.
>
> > +(define-public emacs-interleave
> > +(package
>
> Indentation seems wrong.
>
> > -   (name "emacs-interleave")
> > -   (version "20191129.958")
> > -   (source
> > -   (origin
> > -        (method url-fetch)
> >
> >
> > -        (uri (string-append
> >
> >
> > -               "https://melpa.org/packages/interleave-"
> >
> >
> > -               version
> >
> >
> > -               ".el"))
> >
> >
>
> We do not use MELPA sources. Instead, you could use `git-fetch' and
> extract the latest stable release from here:
>
> https://github.com/rudolfochrist/interleave/releases
>
> > -        (sha256
> >
> >
> > -          (base32
> >
> >
> > -            "0y183ggfmkjjywbcqgyinmr21hsg2vaw4d52bg1gsxbp673za2m8"))))
> >
> >
>
> Nitpick: could you move the string on the same line as `base32'?
>
> > -   (build-system emacs-build-system)
> > -   (home-page
> > -   "https://github.com/rudolfochrist/interleave")
>
> Ditto. Could you move the string on the line above?
>
> > -   (synopsis "Interleaving text books since 2015")
>
> I think this synopsis needs to be reworded. As it is, it may not be
> useful.
>
> > -   (description
> > -   "Interleave presents your PDF side by side to an [[http://orgmode.org][Org Mode]]
>
> Description uses Texinfo syntax, not Org syntax. In this case, however,
> I think "side by side to an Org buffer" is sufficient.
>
> > +buffer with your notes, narrowing down to just those passages that are relevant to
> > +the particular page in the document viewer.
> > +")
>
> You don't need to end the description with a final newline. It would be
> nice to expound it a bit. For example, it could be interesting to know
> this is a minor mode.
>
> Don't forget to add a copyright line with your name at the top of the file.
>
> Could you send an updated patch?
>
> Regards,
>
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Nicolas Goaziou

Comments

Nicolas Goaziou May 17, 2020, 11 p.m. UTC | #1 
Hello,

rdes <rdes@protonmail.com> writes:

> Subject: [PATCH] * gnu/packages/emacs-xyz.scm (emacs-interleave): New
>  variable.

Thank you for the updated patch. Some comments follow.

> +(define-public emacs-interleave
> +  (package
> +    (name "emacs-interleave")
> +    (version "1.4.0")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri
> +        (git-reference
> +         (url "https://github.com/rudolfochrist/interleave")
> +         (commit (string-append "interleave-" version))))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32
> +         "0l5b681mrpk12lx5c16m7kc13p29z6zismwg1llsjg7cdmrmsrcb"))))

Could you move the hash on the line above?

> +    (build-system emacs-build-system)
> +    (home-page
> +     "https://github.com/rudolfochrist/interleave")

Could you move the URI on the line above, too?

> +    (synopsis
> +     "A minor mode that presents a document buffer next to an orgmode buffer")

A synopsis does not start with "A". You may want to use "guix lint" to
spot this. 

You might use the summary from the home page:

  (synopsis "Emacs minor mode to interleave notes and text books")

> +    (description
> +     "Interleave is a minor mode that presents a document viewer side
> +by side to an orgmode buffer with your notes relevant to the current
> page.")

"orgmode buffer" -> "an Org buffer"

Could you send an updated patch and merge this with the copyright line
you sent?

Regards,
diff mbox series

Patch

From 28d87ed5f3d7372a759475b28bc9993560953d2d Mon Sep 17 00:00:00 2001
From: Ryan Desfosses <ryan@desfo.org>
Date: Fri, 15 May 2020 16:53:02 -0400
Subject: [PATCH] * gnu/packages/emacs-xyz.scm (emacs-interleave): New
 variable.

---
 gnu/packages/emacs-xyz.scm | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 45ccced99e..b70b9fffb8 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -6052,6 +6052,31 @@  the Hydra very seamless; it's like a minor mode that disables itself
 automatically.")
     (license license:gpl3+)))
 
+(define-public emacs-interleave
+  (package
+    (name "emacs-interleave")
+    (version "1.4.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri
+        (git-reference
+         (url "https://github.com/rudolfochrist/interleave")
+         (commit (string-append "interleave-" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "0l5b681mrpk12lx5c16m7kc13p29z6zismwg1llsjg7cdmrmsrcb"))))
+    (build-system emacs-build-system)
+    (home-page
+     "https://github.com/rudolfochrist/interleave")
+    (synopsis
+     "A minor mode that presents a document buffer next to an orgmode buffer")
+    (description
+     "Interleave is a minor mode that presents a document viewer side
+by side to an orgmode buffer with your notes relevant to the current page.")
+    (license license:gpl3+)))
+
 (define-public emacs-ivy
   (package
     (name "emacs-ivy")
-- 
2.26.2