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

  • Done
  • quality assurance status badge
Details
2 participants
  • Nicolas Goaziou
  • Sergiu Ivanov
Owner
unassigned
Submitted by
Sergiu Ivanov
Severity
normal

Debbugs page

Sergiu Ivanov wrote 2 years ago
(address . guix-patches@gnu.org)
87v8ncbp09.fsf@colimite.fr
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
From 6f53bc504923028c9cae3619192db976a25af4a4 Mon Sep 17 00:00:00 2001
From: Sergiu Ivanov <sivanov@colimite.fr>
Date: Fri, 18 Nov 2022 10:11:53 +0100
Subject: [PATCH] gnu: Add emacs-org-tree-slide.

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

Toggle diff (42 lines)
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index fe0d9f1dc9..33158f54eb 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -118,6 +118,7 @@
;;; Copyright © 2022 Hilton Chain <hako@ultrarare.space>
;;; Copyright © 2022 Nicolas Graves <ngraves@ngraves.fr>
;;; Copyright © 2022 Thiago Jung Bauermann <bauermann@kolabnow.com>
+;;; Copyright © 2022 Sergiu Ivanov <sivanov@colimite.fr>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -5900,6 +5901,27 @@ (define-public emacs-stripe-buffer
tables.")
(license license:gpl2+)))
+(define-public emacs-org-tree-slide
+ (package
+ (name "emacs-org-tree-slide")
+ (version "20221016.1623")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append "https://melpa.org/packages/org-tree-slide-"
+ version ".el"))
+ (sha256
+ (base32 "0pzq43l80i7p1w0ph5az1nxpwpl50ahmi7ql22ai31x7rh1a44fi"))))
+ (build-system emacs-build-system)
+ (home-page "https://github.com/takaxp/org-tree-slide")
+ (synopsis "Emacs minor mode for giving presentations with 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.")
+ (license license:gpl3)))
+
(define-public emacs-org-beautify-theme
;; Latest release (0.4) is not tagged, use commit hash.
(let ((commit "df6a1114fda313e1689363e196c8284fbe2a2738")
--
2.38.1
Nicolas Goaziou wrote 2 years ago
(name . Sergiu Ivanov)(address . sivanov@colimite.fr)(address . 59352@debbugs.gnu.org)
87mt8oot15.fsf@nicolasgoaziou.fr
Hello,

Sergiu Ivanov <sivanov@colimite.fr> writes:

Toggle quote (2 lines)
> Here's a patch adding emacs-org-tree-slide.

Thank you.

Toggle quote (4 lines)
> 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.

Toggle quote (6 lines)
> 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.


Toggle quote (5 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.

Toggle quote (6 lines)
> + (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.

Toggle quote (2 lines)
> + (synopsis "Emacs minor mode for giving presentations with Org-mode")

Nitpick: Org-mode -> Org mode.

Toggle quote (6 lines)
> + (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.

Toggle quote (2 lines)
> + (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
Sergiu Ivanov wrote 2 years ago
(name . Nicolas Goaziou)(address . mail@nicolasgoaziou.fr)(address . 59352@debbugs.gnu.org)
874juvg6mc.fsf@colimite.fr
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]:
Toggle quote (14 lines)
> 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 :'(

Toggle quote (10 lines)
>> 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.

Toggle quote (17 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!

Toggle quote (4 lines)
>> + (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.

Toggle quote (12 lines)
>> + (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.

Toggle quote (5 lines)
>> + (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.

Toggle quote (4 lines)
> Could you send an updated patch?
>
> Well done BTW!

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

-
Sergiu
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(-)

Toggle diff (42 lines)
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
Nicolas Goaziou wrote 2 years ago
(name . Sergiu Ivanov)(address . sivanov@colimite.fr)(address . 59352-done@debbugs.gnu.org)
87a64np9mv.fsf@nicolasgoaziou.fr
Hello,

Sergiu Ivanov <sivanov@colimite.fr> writes:

Toggle quote (3 lines)
> 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.

Toggle quote (4 lines)
> 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.

Toggle quote (17 lines)
> ---
> 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
Closed
Sergiu Ivanov wrote 2 years ago
(name . Nicolas Goaziou)(address . mail@nicolasgoaziou.fr)(address . 59352-done@debbugs.gnu.org)
87k03rdwac.fsf@colimite.fr
Hello,

Nicolas Goaziou <mail@nicolasgoaziou.fr> [2022-11-19T10:38:00+0100]:
Toggle quote (9 lines)
> 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!

Toggle quote (7 lines)
>> 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.

Toggle quote (4 lines)
> 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.

Toggle quote (22 lines)
>> ---
>> 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
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 59352@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 59352
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help