all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
@ 2022-11-18  9:15 Sergiu Ivanov
  2022-11-18 21:24 ` Nicolas Goaziou
  0 siblings, 1 reply; 5+ messages in thread
From: Sergiu Ivanov @ 2022-11-18  9:15 UTC (permalink / raw)
  To: 59352

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

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

[-- Attachment #2: 0001-gnu-Add-emacs-org-tree-slide.patch --]
[-- Type: text/x-patch, Size: 1976 bytes --]

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

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
  2022-11-18  9:15 [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide Sergiu Ivanov
@ 2022-11-18 21:24 ` Nicolas Goaziou
  2022-11-18 23:54   ` Sergiu Ivanov
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Goaziou @ 2022-11-18 21:24 UTC (permalink / raw)
  To: Sergiu Ivanov; +Cc: 59352

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.

> 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
  2022-11-18 21:24 ` Nicolas Goaziou
@ 2022-11-18 23:54   ` Sergiu Ivanov
  2022-11-19  9:38     ` bug#59352: " Nicolas Goaziou
  0 siblings, 1 reply; 5+ messages in thread
From: Sergiu Ivanov @ 2022-11-18 23:54 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: 59352

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-emacs-org-tree-slide-Update-to-2.8.18.patch --]
[-- Type: text/x-patch, Size: 2157 bytes --]

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#59352: [PATCH] gnu: Add emacs-org-tree-slide.
  2022-11-18 23:54   ` Sergiu Ivanov
@ 2022-11-19  9:38     ` Nicolas Goaziou
  2022-11-19 11:18       ` [bug#59352] " Sergiu Ivanov
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Goaziou @ 2022-11-19  9:38 UTC (permalink / raw)
  To: Sergiu Ivanov; +Cc: 59352-done

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,
-- 
Nicolas Goaziou




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide.
  2022-11-19  9:38     ` bug#59352: " Nicolas Goaziou
@ 2022-11-19 11:18       ` Sergiu Ivanov
  0 siblings, 0 replies; 5+ messages in thread
From: Sergiu Ivanov @ 2022-11-19 11:18 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: 59352-done

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




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-11-19 11:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-18  9:15 [bug#59352] [PATCH] gnu: Add emacs-org-tree-slide Sergiu Ivanov
2022-11-18 21:24 ` Nicolas Goaziou
2022-11-18 23:54   ` Sergiu Ivanov
2022-11-19  9:38     ` bug#59352: " Nicolas Goaziou
2022-11-19 11:18       ` [bug#59352] " Sergiu Ivanov

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.