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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59352 in the body.
You can then email your comments to 59352 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sergiu Ivanov <sivanov <at> colimite.fr>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add emacs-org-tree-slide.
Date: Fri, 18 Nov 2022 10:15:02 +0100

[Message part 1 (text/plain, inline)]

Hello,

Here's a patch adding emacs-org-tree-slide.

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

-
Sergiu

[0001-gnu-Add-emacs-org-tree-slide.patch (text/x-patch, attachment)]

Message #8 received at 59352 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Sergiu Ivanov <sivanov <at> colimite.fr>
Cc: 59352 <at> debbugs.gnu.org
Subject: Re: [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
Date: Fri, 18 Nov 2022 22:24:22 +0100

Hello,

Sergiu Ivanov <sivanov <at> 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.

> 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.


> +(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.

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

Nitpick: Org-mode -> Org mode.

> +    (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.

> +    (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".

Could you send an updated patch?

Well done BTW!

Regards,
-- 
Nicolas Goaziou

Message #11 received at 59352 <at> debbugs.gnu.org (full text, mbox):

From: Sergiu Ivanov <sivanov <at> colimite.fr>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Cc: 59352 <at> debbugs.gnu.org
Subject: Re: [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
Date: Sat, 19 Nov 2022 00:54:50 +0100

[Message part 1 (text/plain, inline)]

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 <at> nicolasgoaziou.fr> [2022-11-18T22:24:22+0100]:
> Hello,
>
> Sergiu Ivanov <sivanov <at> 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

[0001-gnu-emacs-org-tree-slide-Update-to-2.8.18.patch (text/x-patch, attachment)]

Message #16 received at 59352-done <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Sergiu Ivanov <sivanov <at> colimite.fr>
Cc: 59352-done <at> debbugs.gnu.org
Subject: Re: [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
Date: Sat, 19 Nov 2022 10:38:00 +0100

Hello,

Sergiu Ivanov <sivanov <at> 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,
-- 
Nicolas Goaziou

Message #19 received at 59352-done <at> debbugs.gnu.org (full text, mbox):

From: Sergiu Ivanov <sivanov <at> colimite.fr>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Cc: 59352-done <at> debbugs.gnu.org
Subject: Re: [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
Date: Sat, 19 Nov 2022 12:18:07 +0100

Hello,

Nicolas Goaziou <mail <at> nicolasgoaziou.fr> [2022-11-19T10:38:00+0100]:
> Hello,
>
> Sergiu Ivanov <sivanov <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.