guix-patches
[Top][All Lists]
Advanced

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

[bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.


From: Sergiu Ivanov
Subject: [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
Date: Sat, 19 Nov 2022 00:54:50 +0100
User-agent: mu4e 1.8.11; emacs 28.2

Hello Nicolas,

Thank you for your review!

I was starting to apply your suggestions and … I found that
emacs-org-tree-slide was already packaged!  I swear I checked before
sending in the patch, but apparently I didn't check well enough :D :'(

So, I decided to update the existing definition and improve it according
to your suggestions.  I attach the new patch.


Nicolas Goaziou <mail@nicolasgoaziou.fr> [2022-11-18T22:24:22+0100]:
> Hello,
>
> Sergiu Ivanov <sivanov@colimite.fr> writes:
>
>> Here's a patch adding emacs-org-tree-slide.
>
> Thank you.
>
>> It's my second Guix package ever, and I actually enjoyed following the
>> instructions from the manual for building, linting and styling it. Tell
>> me if I got it right :D
>
> Almost ;) Some comments follow.

:D :'(

>> Subject: [PATCH] gnu: Add emacs-org-tree-slide.
>>
>> ---
>>  gnu/packages/emacs-xyz.scm | 22 ++++++++++++++++++++++
>
>
> Your commit message is missing a part about the module being modified:
>
>   * gnu/packages/emacs-xyz.scm (emacs-org-tree-slide): New variable.

A-ha!  I looked at other commit messages, but forgot to not only look at
their first lines.

>> +(define-public emacs-org-tree-slide
>> +  (package
>> +    (name "emacs-org-tree-slide")
>> +    (version "20221016.1623")
>
> Latest version is 2.8.18, the version above is a fancy date tag from
> MELPA unstable.
>
>> +    (source
>> +     (origin
>> +       (method url-fetch)
>> +       (uri (string-append "https://melpa.org/packages/org-tree-slide-";
>> +                           version ".el"))
>
> We don't use MELPA as upstream because it doesn't guarantee the tarball
> will always be available. Use GitHub as upstream instead.

Oh, good to know!

>> +    (synopsis "Emacs minor mode for giving presentations with Org-mode")
>
> Nitpick: Org-mode -> Org mode.

I fixed this in the other package definition which I found.

>> +    (description
>> +     "This package provides the Org minor mode @code{org-tree-slide} which
>> +allows for using an Org-mode document in presentations by
>> +progressively revealing individual subtrees of the document.
>> +org-tree-slide shows and hides parts of the Org buffer by narrowing.")
>
> I suggest:
>
>   Org Tree Slide is a minor mode for using an Org document in
>   presentations by progressively revealing individual subtrees of the
>   document.

I replaced the original text with yours, which I like more.

>> +    (license license:gpl3)))
>
> License is actually gpl3+ because the license in the org-tree-slide.el
> file mention "or (at your option), any later version".

Oh, OK, I'll read better next time.

> Could you send an updated patch?
>
> Well done BTW!

Thank you for your time!  It was a nice and pleasant training.

-
Sergiu

Attachment: 0001-gnu-emacs-org-tree-slide-Update-to-2.8.18.patch
Description: Text Data


reply via email to

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