GNU bug report logs - #39931
[PATCH] gnu: Added emacs-org-journal

Previous Next

Full log

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

From: Martin Becze <mjbecze <at> riseup.net>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Cc: 39931 <at> debbugs.gnu.org
Subject: Re: [bug#39931] [PATCH] gnu: Added emacs-org-journal
Date: Fri, 6 Mar 2020 11:05:07 -0500

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

Thanks for the review Nicolas!

Attached is an updated patch.

On 3/5/20 3:18 PM, Nicolas Goaziou wrote:
> Hello,
> 
> Martin Becze <mjbecze <at> riseup.net> writes:
> 
>> * gnu/packages/emacs-xyz.scm (emacs-org-journal): New Variable.
> 
> Thank you!
> 
>> +(define-public emacs-org-journal
>> +  (package
>> +    (name "emacs-org-journal")
>> +    (version "2.0.0")
>> +    (home-page "https://github.com/bastibe/org-journal")
> 
> Usually, `home-page' is lower in the package definition, right above `synopsis'.
> 
>> +    (source
>> +     (origin
>> +       (method git-fetch)
>> +       (uri (git-reference
>> +             (url home-page)
> 
> It may be just me, but I'm not a big fan of this trick, because it binds
> two information that might be unrelated at some point. I suggest to use
> the full URL instead.
> 
> OTOH, the above is not a strong argument either, so it's up to you.
> 
>> +             (commit version)))
>> +       (file-name (git-file-name name version))
>> +       (sha256
>> +        (base32
>> +         "18dqd0jy2x530lk0h4fcn9cld9qh4w7b3vxa60fpiia628vsv1dg"))))
>> +    (build-system emacs-build-system)
>> +    (synopsis "A simple org-mode based journaling mode")
> 
> I suggest "Simple Org mode based journaling mode". In any case, I think
> "guix lint" complains when a synopsis starts with an article.
> 
>> +    (description "@code{org-journal} maintains a set of files, depending on the value
>> + of @{org-journal-file-type}, a file represents a day, week, month or year. When
>> +@code{org-journal-file-type} is set to =’daily=, each file represent a day. In case
>> +@{org-journal-file-type} is set to =’weekly=, a file represents
>> a week, etc.
> 
> "Org Journal maintains a set of files, which may represent a day,
> a week, a month or a year."
> 
> IOW, I think description should not leak the innards of the package.
> 
>> + Convenient bindings allow the creation of journal records in the current daily,
>> + weekly, monthly or yearly file and search within all records or specified time
>> + intervals. All records can be browsed and searched from the Emacs Calendar for
>> + convenience.")
> 
> You need to add two spaces after full stops.
> 
> Could you send an updated patch?
> 
> Regards,
> 

[v2-0001-gnu-Added-emacs-org-journal.patch (text/x-patch, attachment)]

Previous Next

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