diff mbox series

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

Message ID 874juvg6mc.fsf@colimite.fr
State New
Headers show
Series [bug#59352] gnu: Add emacs-org-tree-slide. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success
cbaines/issue success View issue

Commit Message

Sergiu Ivanov Nov. 18, 2022, 11:54 p.m. UTC 
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

Comments

Nicolas Goaziou Nov. 19, 2022, 9:38 a.m. UTC | #1 
Hello,

Sergiu Ivanov <sivanov@colimite.fr> writes:

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

Great! I applied it with a minor twist explained below.

> Subject: [PATCH] gnu: emacs-org-tree-slide: Update to 2.8.18.
>
> * gnu/packages/emacs-xyz.scm (emacs-org-tree-slide): Update to 2.8.18.

You're updating to the latest commit, which is not exactly "2.8.18", to
"2.8.18-0.d6529bc".

Also, the commit message must include changes you made to synopsis and
description, which could arguably have been done in a subsequent commit,
but that's fine.

> ---
>  gnu/packages/emacs-xyz.scm | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index fe0d9f1dc9..f827107b29 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -18644,11 +18644,11 @@ (define-public emacs-kotlin-mode
>        (license license:gpl3+))))
>  
>  (define-public emacs-org-tree-slide
> -  (let ((commit "036a36eec1cf712d3db155572aed325daa372eb5")
> -        (revision "2"))
> +  (let ((commit "d6529bc2df727d09014e0e56abf4f15a8e8fc20f")
> +        (revision "3"))

The revision is reset to "0" since you bumped the base version. Revision
is here to ensure monotonic growth between version bumps because commit
hashes cannot ensure this. Therefore, it is only useful to increase the
revision number within the same base version.

Thank you!

Regards,
Sergiu Ivanov Nov. 19, 2022, 11:18 a.m. UTC | #2 
Hello,

Nicolas Goaziou <mail@nicolasgoaziou.fr> [2022-11-19T10:38:00+0100]:
> Hello,
>
> Sergiu Ivanov <sivanov@colimite.fr> writes:
>
>> So, I decided to update the existing definition and improve it according
>> to your suggestions.  I attach the new patch.
>
> Great! I applied it with a minor twist explained below.

Great, thank you!

>> Subject: [PATCH] gnu: emacs-org-tree-slide: Update to 2.8.18.
>>
>> * gnu/packages/emacs-xyz.scm (emacs-org-tree-slide): Update to 2.8.18.
>
> You're updating to the latest commit, which is not exactly "2.8.18", to
> "2.8.18-0.d6529bc".

Oh, OK, thank you for the explanation!  I am consistently bad at getting
versioning right.

> Also, the commit message must include changes you made to synopsis and
> description, which could arguably have been done in a subsequent commit,
> but that's fine.

I hesitated about that.  I'll split the changes over two patches the
next time.

>> ---
>>  gnu/packages/emacs-xyz.scm | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
>> index fe0d9f1dc9..f827107b29 100644
>> --- a/gnu/packages/emacs-xyz.scm
>> +++ b/gnu/packages/emacs-xyz.scm
>> @@ -18644,11 +18644,11 @@ (define-public emacs-kotlin-mode
>>        (license license:gpl3+))))
>>  
>>  (define-public emacs-org-tree-slide
>> -  (let ((commit "036a36eec1cf712d3db155572aed325daa372eb5")
>> -        (revision "2"))
>> +  (let ((commit "d6529bc2df727d09014e0e56abf4f15a8e8fc20f")
>> +        (revision "3"))
>
> The revision is reset to "0" since you bumped the base version. Revision
> is here to ensure monotonic growth between version bumps because commit
> hashes cannot ensure this. Therefore, it is only useful to increase the
> revision number within the same base version.

Oh, I see!  I read the docs of git-version to see what "revision" meant,
but I didn't get that revision should be reset to 0 when the base
version is changed.

Thank you for taking the time to review my patch and explain
the details!

-
Sergiu
diff mbox series

Patch

From 97e1307f0a8966149ecf29264ad205e55d29b502 Mon Sep 17 00:00:00 2001
From: Sergiu Ivanov <sivanov@colimite.fr>
Date: Sat, 19 Nov 2022 00:46:04 +0100
Subject: [PATCH] gnu: emacs-org-tree-slide: Update to 2.8.18.

* gnu/packages/emacs-xyz.scm (emacs-org-tree-slide): Update to 2.8.18.
---
 gnu/packages/emacs-xyz.scm | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index fe0d9f1dc9..f827107b29 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -18644,11 +18644,11 @@  (define-public emacs-kotlin-mode
       (license license:gpl3+))))
 
 (define-public emacs-org-tree-slide
-  (let ((commit "036a36eec1cf712d3db155572aed325daa372eb5")
-        (revision "2"))
+  (let ((commit "d6529bc2df727d09014e0e56abf4f15a8e8fc20f")
+        (revision "3"))
     (package
       (name "emacs-org-tree-slide")
-      (version (git-version "2.8.4" revision commit))
+      (version (git-version "2.8.18" revision commit))
       (source (origin
                 (method git-fetch)
                 (uri (git-reference
@@ -18656,15 +18656,15 @@  (define-public emacs-org-tree-slide
                       (commit commit)))
                 (sha256
                  (base32
-                  "1r8ncx25xmxicgciyv5przp68y8qgy40fm10ba55awvql4xcm0yk"))
+                  "1br32mpwarmrn158y2pkkmfl2ssv8q8spzknkg2avr16fil0j1pz"))
                 (file-name (git-file-name name version))))
       (build-system emacs-build-system)
       (home-page "https://github.com/takaxp/org-tree-slide")
-      (synopsis "Presentation tool for org-mode")
+      (synopsis "Presentation tool for Org mode")
       (description
-       "Org-tree-slide provides a slideshow mode to view org-mode files.  Use
-@code{org-tree-slide-mode} to enter the slideshow mode, and then @kbd{C->} and
-@kbd{C-<} to jump to the next and previous slide.")
+       "Org Tree Slide is a minor mode for using an Org document in
+presentations by progressively revealing individual subtrees of
+the document.")
       (license license:gpl3+))))
 
 (define-public emacs-scratch-el
-- 
2.38.1