[PATCH 2/2] gnu: Add emacs-org-trello.

[PATCH 2/2] gnu: Add emacs-org-trello.

[Roel Janssen]

* gnu/packages/emacs.scm (emacs-org-trello): New variable.
---
 gnu/packages/emacs.scm | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 69fd10f..964ba59 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -2086,6 +2086,32 @@ number.")
 @code{org-mode} to be rendered as UTF-8 characters.")
     (license license:gpl3+)))
 
+(define-public emacs-org-trello
+  (package
+    (name "emacs-org-trello")
+    (version "0.7.9")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/org-trello/org-trello/archive/"
+                    version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf"))))
+    (build-system emacs-build-system)
+    (propagated-inputs
+     `(("emacs-deferred" ,emacs-deferred)
+       ("emacs-request" ,emacs-request)
+       ("emacs-dash" ,emacs-dash)
+       ("emacs-s" ,emacs-s)))
+    (home-page "https://org-trello.github.io")
+    (synopsis "Emacs minor mode for interacting with Trello")
+    (description "This package provides an emacs minor mode to extend
+@code{org-mode} with Trello abilities.  Trello is an online project
+organizer.")
+    (license license:gpl3+)))
+
 (define-public emacs-zenburn-theme
   (package
     (name "emacs-zenburn-theme")
-- 
2.10.0

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Ricardo Wurmus]

Hi Roel,

funny, this has also been on my pile of unsubmitted, bit-rotting patches
(because I must use Trello at work and I much rather use org-mode).  I
remember that this wasn’t straight-forward to package and required a lot
more (build-time) inputs.

The cask file says:

(development
 (depends-on "dash-functional")
 (depends-on "dash")
 (depends-on "ert-runner")
 (depends-on "undercover")
 (depends-on "ert")
 (depends-on "ert-expectations")
 (depends-on "el-mock")
 (depends-on "helm"))

Should these be added?

> * gnu/packages/emacs.scm (emacs-org-trello): New variable.
> ---
>  gnu/packages/emacs.scm | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index 69fd10f..964ba59 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -2086,6 +2086,32 @@ number.")
>  @code{org-mode} to be rendered as UTF-8 characters.")
>      (license license:gpl3+)))
>  
> +(define-public emacs-org-trello
> +  (package
> +    (name "emacs-org-trello")
> +    (version "0.7.9")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://github.com/org-trello/org-trello/archive/"
> +                    version ".tar.gz"))
> +              (file-name (string-append name "-" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf"))))
> +    (build-system emacs-build-system)
> +    (propagated-inputs
> +     `(("emacs-deferred" ,emacs-deferred)
> +       ("emacs-request" ,emacs-request)
> +       ("emacs-dash" ,emacs-dash)
> +       ("emacs-s" ,emacs-s)))
> +    (home-page "https://org-trello.github.io")
> +    (synopsis "Emacs minor mode for interacting with Trello")
> +    (description "This package provides an emacs minor mode to extend

Please capitalize “Emacs”.

> +@code{org-mode} with Trello abilities.  Trello is an online project
> +organizer.")
> +    (license license:gpl3+)))
> +
>  (define-public emacs-zenburn-theme
>    (package
>      (name "emacs-zenburn-theme")

Other than that it looks good!

Thanks!

~~ Ricardo

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Roel Janssen]

Ricardo Wurmus writes:

> Hi Roel,
>
> funny, this has also been on my pile of unsubmitted, bit-rotting patches
> (because I must use Trello at work and I much rather use org-mode).  I
> remember that this wasn’t straight-forward to package and required a lot
> more (build-time) inputs.

We seem to have quite some overlap between our packages and preference
for the software programs we want to use. :)

> The cask file says:
>
> (development
>  (depends-on "dash-functional")
>  (depends-on "dash")

@code{dash} provides @code{dash-functional}.

>  (depends-on "undercover")

This one is needed to run the tests.

>  (depends-on "ert-runner")
>  (depends-on "ert")
>  (depends-on "ert-expectations")

These should be built-in.  See:
https://www.gnu.org/software/emacs/manual/html_node/ert/

(Or M-x ert)

>  (depends-on "el-mock")
>  (depends-on "helm"))

These should be added too, to run the tests.

I added undercover, el-mock and helm to the inputs.


>> * gnu/packages/emacs.scm (emacs-org-trello): New variable.
>> ---
>>  gnu/packages/emacs.scm | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
>> index 69fd10f..964ba59 100644
>> --- a/gnu/packages/emacs.scm
>> +++ b/gnu/packages/emacs.scm
>> @@ -2086,6 +2086,32 @@ number.")
>>  @code{org-mode} to be rendered as UTF-8 characters.")
>>      (license license:gpl3+)))
>>  
>> +(define-public emacs-org-trello
>> +  (package
>> +    (name "emacs-org-trello")
>> +    (version "0.7.9")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri (string-append
>> +                    "https://github.com/org-trello/org-trello/archive/"
>> +                    version ".tar.gz"))
>> +              (file-name (string-append name "-" version ".tar.gz"))
>> +              (sha256
>> +               (base32
>> +                "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf"))))
>> +    (build-system emacs-build-system)
>> +    (propagated-inputs
>> +     `(("emacs-deferred" ,emacs-deferred)
>> +       ("emacs-request" ,emacs-request)
>> +       ("emacs-dash" ,emacs-dash)
>> +       ("emacs-s" ,emacs-s)))
>> +    (home-page "https://org-trello.github.io")
>> +    (synopsis "Emacs minor mode for interacting with Trello")
>> +    (description "This package provides an emacs minor mode to extend
>
> Please capitalize “Emacs”.

Whoops.  Fixed in the new patch.

>
>> +@code{org-mode} with Trello abilities.  Trello is an online project
>> +organizer.")
>> +    (license license:gpl3+)))
>> +
>>  (define-public emacs-zenburn-theme
>>    (package
>>      (name "emacs-zenburn-theme")
>
> Other than that it looks good!
>
> Thanks!

There are three failing tests, because it cannot find the test
function definitions of itself, or it cannot find the built-in
functions: hash-table-keys, hash-table-values, and string-at-pt.

I think the tests are malfunctioning, not the functionality, because I
can synchronize my Trello boards and lists just fine.

Here's the updated patch:

From de2e2f3a5b293d75b5e65fc3081a2ead75149e4c Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Thu, 27 Oct 2016 10:28:51 +0200
Subject: [PATCH] gnu: Add emacs-org-trello.

* gnu/packages/emacs.scm (emacs-org-trello): New variable.
---
 gnu/packages/emacs.scm | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 2a275d0..6af1325 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -2086,6 +2086,35 @@ number.")
 @code{org-mode} to be rendered as UTF-8 characters.")
     (license license:gpl3+)))
 
+(define-public emacs-org-trello
+  (package
+    (name "emacs-org-trello")
+    (version "0.7.9")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/org-trello/org-trello/archive/"
+                    version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf"))))
+    (build-system emacs-build-system)
+    (propagated-inputs
+     `(("emacs-deferred" ,emacs-deferred)
+       ("emacs-request" ,emacs-request)
+       ("emacs-dash" ,emacs-dash)
+       ("emacs-s" ,emacs-s)
+       ("emacs-undercover" ,emacs-undercover)
+       ("emacs-el-mock" ,emacs-el-mock)
+       ("emacs-helm" ,emacs-helm)))
+    (home-page "https://org-trello.github.io")
+    (synopsis "Emacs minor mode for interacting with Trello")
+    (description "This package provides an Emacs minor mode to extend
+@code{org-mode} with Trello abilities.  Trello is an online project
+organizer.")
+    (license license:gpl3+)))
+
 (define-public emacs-zenburn-theme
   (package
     (name "emacs-zenburn-theme")
-- 
2.10.0


Kind regards,
Roel Janssen

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Ricardo Wurmus]

Roel Janssen <roel@gnu.org> writes:

>> The cask file says:
>>
>> (development
>>  (depends-on "dash-functional")
>>  (depends-on "dash")
>
> @code{dash} provides @code{dash-functional}.
>
>>  (depends-on "undercover")
>
> This one is needed to run the tests.
>
>>  (depends-on "ert-runner")
>>  (depends-on "ert")
>>  (depends-on "ert-expectations")
>
> These should be built-in.  See:
> https://www.gnu.org/software/emacs/manual/html_node/ert/
>
> (Or M-x ert)
>
>>  (depends-on "el-mock")
>>  (depends-on "helm"))
>
> These should be added too, to run the tests.
>
> I added undercover, el-mock and helm to the inputs.

[…]

> +(define-public emacs-org-trello
> +  (package
> +    (name "emacs-org-trello")
> +    (version "0.7.9")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://github.com/org-trello/org-trello/archive/"
> +                    version ".tar.gz"))
> +              (file-name (string-append name "-" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf"))))
> +    (build-system emacs-build-system)
> +    (propagated-inputs
> +     `(("emacs-deferred" ,emacs-deferred)
> +       ("emacs-request" ,emacs-request)
> +       ("emacs-dash" ,emacs-dash)
> +       ("emacs-s" ,emacs-s)
> +       ("emacs-undercover" ,emacs-undercover)
> +       ("emacs-el-mock" ,emacs-el-mock)
> +       ("emacs-helm" ,emacs-helm)))

Hmm, now I wonder… I just looked at emacs.scm and only one package there
has “emacs-undercover” as an input (and it looks like this was a
mistake).  We seem not to add test inputs to Emacs packages.

@Alex: could you please confirm this?  Should these inputs be mentioned
at all?  If so, is it sufficient to add them to native-inputs?

~~ Ricardo

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Alex Kost]

Ricardo Wurmus (2016-10-27 16:09 +0200) wrote:

> Roel Janssen <roel@gnu.org> writes:
[...]
>> +(define-public emacs-org-trello
>> +  (package
>> +    (name "emacs-org-trello")
>> +    (version "0.7.9")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri (string-append
>> +                    "https://github.com/org-trello/org-trello/archive/"
>> +                    version ".tar.gz"))
>> +              (file-name (string-append name "-" version ".tar.gz"))
>> +              (sha256
>> +               (base32
>> +                "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf"))))
>> +    (build-system emacs-build-system)
>> +    (propagated-inputs
>> +     `(("emacs-deferred" ,emacs-deferred)
>> +       ("emacs-request" ,emacs-request)
>> +       ("emacs-dash" ,emacs-dash)
>> +       ("emacs-s" ,emacs-s)
>> +       ("emacs-undercover" ,emacs-undercover)
>> +       ("emacs-el-mock" ,emacs-el-mock)
>> +       ("emacs-helm" ,emacs-helm)))
>
> Hmm, now I wonder… I just looked at emacs.scm and only one package there
> has “emacs-undercover” as an input (and it looks like this was a
> mistake).  We seem not to add test inputs to Emacs packages.
>
> @Alex: could you please confirm this?  Should these inputs be mentioned
> at all?  If so, is it sufficient to add them to native-inputs?

As for me, I don't see a reason to add these additional inputs,
emacs-build-system will not run tests anyway.  I think it is good to
stick only to those dependencies that are listed in a header commentary
(in "org-trello.el") or in a "-pkg.el" file if it presents
("org-trello-pkg.el") – these are the exact dependencies that are used
by (M)ELPA.

I wouldn't bother about the rest emacs packages that are listed in
"Cask" file or are used in some other way by package developers; they
don't add anything useful for the end users.

So I think the original Roel's variant with 4 propagated-inputs is
preferable.  The rest dependencies ("undercover", "el-mock" and "helm")
are used only for some developer tools which are not interesting for
users, these tools are even excluded in the MELPA recipe:

<https://github.com/melpa/melpa/blob/master/recipes/org-trello>

-- 
Alex

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Ricardo Wurmus]

Alex Kost <alezost@gmail.com> writes:

>> @Alex: could you please confirm this?  Should these inputs be mentioned
>> at all?  If so, is it sufficient to add them to native-inputs?
>
> As for me, I don't see a reason to add these additional inputs,
> emacs-build-system will not run tests anyway.

Thank you for clarifying.

Is there a reason why we don’t run the tests?  We do this by default for
all other packages.

~~ Ricardo

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Alex Kost]

Ricardo Wurmus (2016-10-27 20:48 +0200) wrote:

> Alex Kost <alezost@gmail.com> writes:
>
>>> @Alex: could you please confirm this?  Should these inputs be mentioned
>>> at all?  If so, is it sufficient to add them to native-inputs?
>>
>> As for me, I don't see a reason to add these additional inputs,
>> emacs-build-system will not run tests anyway.
>
> Thank you for clarifying.
>
> Is there a reason why we don’t run the tests?  We do this by default for
> all other packages.

How can it be done?  There is no common way for emacs packages to
perform tests: only some packages have Makefile (with "check" phase),
and it would be a really good luck if "make check" as is succeeded for
them.  Usually such projects use Cask to run tests or even some custom
scripts.  They may use just "ert" or "ert-running" package (or maybe
something else).

After all, I think that adding a 'check' phase to emacs-build-system is
not worth an effort: there would be too much manual interventions in
package recipes to make tests work for different emacs packages.

But I better let Federico speak about «why we don’t run the tests» as he
wrote emacs-build-system :-)

-- 
Alex

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Roel Janssen]

Alex Kost writes:

> Ricardo Wurmus (2016-10-27 20:48 +0200) wrote:
>
>> Alex Kost <alezost@gmail.com> writes:
>>
>>>> @Alex: could you please confirm this?  Should these inputs be mentioned
>>>> at all?  If so, is it sufficient to add them to native-inputs?
>>>
>>> As for me, I don't see a reason to add these additional inputs,
>>> emacs-build-system will not run tests anyway.
>>
>> Thank you for clarifying.
>>
>> Is there a reason why we don’t run the tests?  We do this by default for
>> all other packages.
>
> How can it be done?  There is no common way for emacs packages to
> perform tests: only some packages have Makefile (with "check" phase),
> and it would be a really good luck if "make check" as is succeeded for
> them.  Usually such projects use Cask to run tests or even some custom
> scripts.  They may use just "ert" or "ert-running" package (or maybe
> something else).
>
> After all, I think that adding a 'check' phase to emacs-build-system is
> not worth an effort: there would be too much manual interventions in
> package recipes to make tests work for different emacs packages.
>
> But I better let Federico speak about «why we don’t run the tests» as he
> wrote emacs-build-system :-)

So, the only proper way to do it, is to manually add a test phase in
each package definition.  But then we still have to add a lot of
dependencies that are only needed to run the tests, and even then the
tests don't say much..  In the case of this package, the test functions
can't run because they use functions that aren't loaded yet, which
results in failures of the tests (while the package works just fine).

Could I therefore, please, go ahead and push the patch without the
additional inputs for running tests?

Kind regards,
Roel Janssen

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Federico Beffa]

On Fri, Oct 28, 2016 at 11:30 AM, Alex Kost <alezost@gmail.com> wrote:
> Ricardo Wurmus (2016-10-27 20:48 +0200) wrote:
>
>> Alex Kost <alezost@gmail.com> writes:
>>
>>>> @Alex: could you please confirm this?  Should these inputs be mentioned
>>>> at all?  If so, is it sufficient to add them to native-inputs?
>>>
>>> As for me, I don't see a reason to add these additional inputs,
>>> emacs-build-system will not run tests anyway.
>>
>> Thank you for clarifying.
>>
>> Is there a reason why we don’t run the tests?  We do this by default for
>> all other packages.
>
> How can it be done?  There is no common way for emacs packages to
> perform tests: only some packages have Makefile (with "check" phase),
> and it would be a really good luck if "make check" as is succeeded for
> them.  Usually such projects use Cask to run tests or even some custom
> scripts.  They may use just "ert" or "ert-running" package (or maybe
> something else).
>
> After all, I think that adding a 'check' phase to emacs-build-system is
> not worth an effort: there would be too much manual interventions in
> package recipes to make tests work for different emacs packages.

You said it all!

Fede

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Ricardo Wurmus]

Roel Janssen <roel@gnu.org> writes:

> So, the only proper way to do it, is to manually add a test phase in
> each package definition.  But then we still have to add a lot of
> dependencies that are only needed to run the tests, and even then the
> tests don't say much..  In the case of this package, the test functions
> can't run because they use functions that aren't loaded yet, which
> results in failures of the tests (while the package works just fine).
>
> Could I therefore, please, go ahead and push the patch without the
> additional inputs for running tests?

Yes, please do!  I was just curious.  Sorry for delaying this!

~~ Ricardo

Re: [PATCH 2/2] gnu: Add emacs-org-trello.

[Roel Janssen]

Ricardo Wurmus writes:

> Roel Janssen <roel@gnu.org> writes:
>
>> So, the only proper way to do it, is to manually add a test phase in
>> each package definition.  But then we still have to add a lot of
>> dependencies that are only needed to run the tests, and even then the
>> tests don't say much..  In the case of this package, the test functions
>> can't run because they use functions that aren't loaded yet, which
>> results in failures of the tests (while the package works just fine).
>>
>> Could I therefore, please, go ahead and push the patch without the
>> additional inputs for running tests?
>
> Yes, please do!  I was just curious.  Sorry for delaying this!
>
> ~~ Ricardo

Pushed in 36e5d1038 and ed8bc028f.

Thanks!