Subject: Re: [bug#74051] [PATCH] gnu: Add emacs-weblorg

From: Mattia Bunel <[email-masked]>
Date: Mon, 28 Oct 2024 13:08:22 +0100

Hello Nicolas,

Thanks for your review.

I've written a new patch based on your comments. What is the correct 
procedure for sending it? Should I send a patch based on *origin/master* 
or the previous patch you reviewed?

For documentation, the weblorg *doc/* folder contains the sources for 
the project website. My opinion is therefore not to include them.

Regards,

Le 28/10/2024 à 00:18, Nicolas Goaziou a écrit :
> Hello,
> 
> Mattia Bunel <[email-masked]> writes:
> 
>> Change-Id: Iae2d6cbbadec7a171e1e5b8502d4f47701d7af5b
> 
> Thank you for your patch. There are some (small) things to fix before it
> can be merged, tho.
> 
> First you need to write a proper commit message, such as:
> 
>    gnu: Add emacs-weblorg.
> 
>    * gnu/packages/emacs-xyz.scm (emacs-weblorg): New variable.
> 
>> +(define-public emacs-weblorg
>> +  (package
>> +    (name "emacs-weblorg")
>> +    (version "0.1.2")
> 
> The version is not actually "0.1.2", or rather, it seems we need to pick
> a commit after this tag. You should let-bind the commit hash, i.e.,
> "0db218bd6b2e083546d3a69a022dfb1a08900acd", to a `commit' symbol.
> Then, `version' field would become:
> 
>    (version (git-version "0.1.2" revision commit))
> 
> See, e.g., `emacs-taskrunner' package definition.
> 
>> +    (source
>> +     (origin
>> +       (method url-fetch)
>> +       (uri (string-append "https://stable.melpa.org/packages/" "weblorg-"
>> +                           version ".tar"))
> 
> Upstream is on GitHub, not on Stable MELPA. The package should use
> `git-fetch' method, and
> 
>    (uri (git-reference
>          (url "https://github.com/emacs-love/weblorg")
>          (commit commit)))
> 
> as the `uri' field.
> 
> Also, you need to add
> 
>    (file-name (git-file-name name version))
> 
> before the `sha256' field.
> 
>> +       (sha256
>> +        (base32 "0lvjfhysiyaz8klbj01da5jdjw1p4vvd9kl1zmi9vciazayh82ch"))))
>> +    (build-system emacs-build-system)
> 
> There are tests. You may want to run them, probably with
> 
>    (arguments
>     (list #:tests? #t
>           #:test-command '("emacs" "--batch"
>                            "-l" "t/weblorg-tests.el"
>                            "-f" "ert-run-tests-batch-and-exit")))
> 
> There seem to be some documentation in the "doc/" directory. What should
> be done about it?
> 
>> +    (propagated-inputs (list emacs-templatel))
>> +    (home-page "https://emacs.love/weblorg")
>> +    (synopsis "Static site generator for org-mode")
> 
> Nitpick: org-mode -> Org mode
> 
>> +    (description
>> +     "@code{emacs-weblorg} is a static site generator for org-mode,
>> +based on @code{templatel} library.")
> 
> Nitpick: @code{emacs-weblorg} -> Weblorg, org-mode -> Org mode. I also
> suggest to remove the reference about Templatel library in the
> description.
> 
> Could you send an updated patch? If you have any trouble doing so,
> please let us know.
> 
> Regards,