[bug#50333] [PATCH] Fixed missing files in org-roam 2.1.0

[zimoun]

Hi,

On Tue, 14 Sep 2021 at 17:10, Adolfo De Unánue <adolfo@unanue.mx> wrote:
> zimoun <zimon.toutoune@gmail.com> writes:

>>> Subject: [PATCH] Fixed missing files in org-roam v2.1.0
>>
>> The commit message should be:
>>
>> --8<---------------cut  here---------------start------------->8---
>> gnu: emacs-org-roam: Adding extensions.
>>
>> * guix/emacs-xyz.scm (emacs-org-roam)[arguments]: Add phases to 
>> install
>> extensions.
>> --8<---------------cut here---------------end--------------->8---
>>
>> Or something along these lines.
>
> Should I apply your suggested changes, commit them, use this 
> commit
> message, create the patch and send them again?

Yeah, you can address and send then an updated patch.  Doing so, please
increment the re-roll count which eases to know that patch is the last.

For instance, the ones you sent looks like:

--8<---------------cut here---------------start------------->8---
[ Adolfo De Unánue ] [bug#50333] [PATCH] Fixed missing files in org-roam 2.1.0
    [ Adolfo De Unánue ] 
    [ Adolfo De Unánue ] 
--8<---------------cut here---------------end--------------->8---

and the reviewer has to open all 3 to know what happens.  Compare with
for instance:

--8<---------------cut here---------------start------------->8---
[ Marius Bakke ] [bug#50377] [PATCH 0/2] Support 'git describe'  …
    [ Marius Bakke ] [bug#50377] [PATCH 1/2] git: 'resolve-refer …
        [ Marius Bakke ] [bug#50377] [PATCH 2/2] transformations …
        [ Marius Bakke ] [bug#50377] [PATCH v2 1/2] git: 'resolv …
    [ Marius Bakke ] [bug#50377] [PATCH v3 0/2] Support 'git des …
        [ Marius Bakke ] [bug#50377] [PATCH v3 1/2] git: 'resolv …
            [ Ludovic Courtès ] [bug#50377] [PATCH 0/2] Support  …
        [ Marius Bakke ] [bug#50377] [PATCH v3 2/2] transformati …
            [ Ludovic Courtès ] [bug#50377] [PATCH 0/2] Support  …
        [ Xinglu Chen ] [bug#50377] [PATCH v3 0/2] Support 'git  …
--8<---------------cut here---------------end--------------->8---

Well, I suggest you to use “git format-patch -v4”. :-)


>>> +  (let ((commit "f819720c510185af713522c592833ec9f2934251")
>>
>> Usually, the package uses tagged version instead of random 
>> commit.  When
>> it is not possible because upstream do not tag, it seems good to 
>> provide
>> an explanation why the lasted tagged version cannot be used;
>> explanations as a comment or in the cover letter.
>>
>
> As you guessed, org-roam has not tagged this new (breaking) 
> change,
> that's why I am using the commit

Yeah, but is it released?  Or simply the development version?  If it is
the development version, usually it is on per user basis, i.e., you can
use a package transformation.  Inclusion of releases is preferred.

>>> +           (add-after 'install 'install-extensions
>>> +             (lambda* (#:key outputs #:allow-other-keys)
>>> +               (copy-recursively
>>> +                "extensions"
>>> +                (string-append (assoc-ref outputs "out")
>>> +                               (string-append
>>> +                                "/share/emacs/site-lisp/org-roam-"
>>> +                                ,version)))
>>> +               #t))
>>
>> Patch#50333 will install the extensions with “guix install
>> emacs-org-roam”.  Instead the attempt of patch#50374 is to 
>> install with
>> “guix install emacs-org-roam-extensions”.  Well, I do not know 
>> if
>> patch#50374 is correct, though.
>>
>> Because I am not an Org-Roam user, I do not have an opinion 
>> about this
>> extensions.  Maybe it is better to distribute them along the 
>> package
>> emacs-org-roam or in a separate package, I do not know.  WDYT?
>
> I mean, structurally the upstream owner just moved some files to a 
> new
> folder, my guess is that it shouldn't be a different packages 
> since it
> is still part of org-roam.

Note that some packages are only one upstream source but indeed
different packages.  For instance, notmuch and emacs-notmuch (for the
most recent :-)).  Another example, IIRC, some time ago, Org-contrib was
a folder under Org but distributed by 2 packages.  Well, it is sometime
the same package and simply that upstream reorganize stuff. :-)

BTW, why is it needed to explicitly copy the “extensions” files?
Without this added phase, does it not work?

All the best,
simon