all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#67681] [PATCH] gnu: Add node-commander.
@ 2023-12-07  8:29 Rikard Nordgren
  2024-01-04  3:31 ` Maxim Cournoyer
  0 siblings, 1 reply; 4+ messages in thread
From: Rikard Nordgren @ 2023-12-07  8:29 UTC (permalink / raw)
  To: 67681; +Cc: hrn

* gnu/packages/node-xyz.scm (node-commander): New variable.

Change-Id: I5b00861705d64e0e89da81acee833e145287bd66
---
 gnu/packages/node-xyz.scm | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/packages/node-xyz.scm b/gnu/packages/node-xyz.scm
index 6c16417309..8486d121dd 100644
--- a/gnu/packages/node-xyz.scm
+++ b/gnu/packages/node-xyz.scm
@@ -155,6 +155,44 @@ (define-public node-color-name
      "This package provides a JSON list with color names and their values.")
     (license license:expat)))
 
+(define-public node-commander
+  (package
+    (name "node-commander")
+    (version "11.1.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/tj/commander.js")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "1xwh85kbxj76ni41r2h0apl8mjbfcnmxzzp3vlspq30w8kwfckni"))))
+    (build-system node-build-system)
+    (arguments
+     '(#:tests? #f
+       #:phases (modify-phases %standard-phases
+                  (add-after 'patch-dependencies 'delete-dependencies
+                    (lambda _
+                      (delete-dependencies '("@types/jest" "@types/node"
+                                             "@typescript-eslint/eslint-plugin"
+                                             "@typescript-eslint/parser"
+                                             "eslint"
+                                             "eslint-config-standard"
+                                             "eslint-config-standard-with-typescript"
+                                             "eslint-plugin-import"
+                                             "eslint-plugin-jest"
+                                             "eslint-plugin-n"
+                                             "eslint-plugin-promise"
+                                             "jest"
+                                             "ts-jest"
+                                             "tsd"
+                                             "typescript")))))))
+    (home-page "https://github.com/tj/commander.js")
+    (synopsis "Command line interface for node.js")
+    (description "The complete solution for node.js command-line interfaces.")
+    (license license:expat)))
+
 (define-public node-crx3
   (package
     (name "node-crx3")

base-commit: 23627b1706af25a0a30b96b9169a3495279aff1b
-- 
2.34.1





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

* [bug#67681] [PATCH] gnu: Add node-commander.
  2023-12-07  8:29 [bug#67681] [PATCH] gnu: Add node-commander Rikard Nordgren
@ 2024-01-04  3:31 ` Maxim Cournoyer
  2024-01-04 17:20   ` hrn
  0 siblings, 1 reply; 4+ messages in thread
From: Maxim Cournoyer @ 2024-01-04  3:31 UTC (permalink / raw)
  To: Rikard Nordgren; +Cc: 67681

Hi,

Rikard Nordgren <hrn@posteo.net> writes:

> * gnu/packages/node-xyz.scm (node-commander): New variable.
>
> Change-Id: I5b00861705d64e0e89da81acee833e145287bd66
> ---
>  gnu/packages/node-xyz.scm | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/gnu/packages/node-xyz.scm b/gnu/packages/node-xyz.scm
> index 6c16417309..8486d121dd 100644
> --- a/gnu/packages/node-xyz.scm
> +++ b/gnu/packages/node-xyz.scm
> @@ -155,6 +155,44 @@ (define-public node-color-name
>       "This package provides a JSON list with color names and their values.")
>      (license license:expat)))
>  
> +(define-public node-commander
> +  (package
> +    (name "node-commander")
> +    (version "11.1.0")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/tj/commander.js")
> +             (commit (string-append "v" version))))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32 "1xwh85kbxj76ni41r2h0apl8mjbfcnmxzzp3vlspq30w8kwfckni"))))
> +    (build-system node-build-system)
> +    (arguments
> +     '(#:tests? #f

Why is the test suite disabled?  A comment (explaining a good reason) is
needed :-).

> +       #:phases (modify-phases %standard-phases
> +                  (add-after 'patch-dependencies 'delete-dependencies

A comment explaining why these are removed would help.

> +                    (lambda _
> +                      (delete-dependencies '("@types/jest" "@types/node"
> +                                             "@typescript-eslint/eslint-plugin"
> +                                             "@typescript-eslint/parser"
> +                                             "eslint"
> +                                             "eslint-config-standard"
> +                                             "eslint-config-standard-with-typescript"
> +                                             "eslint-plugin-import"
> +                                             "eslint-plugin-jest"
> +                                             "eslint-plugin-n"
> +                                             "eslint-plugin-promise"
> +                                             "jest"
> +                                             "ts-jest"
> +                                             "tsd"
> +                                             "typescript")))))))
> +    (home-page "https://github.com/tj/commander.js")
> +    (synopsis "Command line interface for node.js")
> +    (description "The complete solution for node.js command-line interfaces.")

Could you please expand a bit on the description?  What features does it
have?  Keep it factual; "The complete solution ..." sounds like
marketing language.

-- 
Thanks,
Maxim




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

* [bug#67681] [PATCH] gnu: Add node-commander.
  2024-01-04  3:31 ` Maxim Cournoyer
@ 2024-01-04 17:20   ` hrn
  2024-01-06 19:05     ` Maxim Cournoyer
  0 siblings, 1 reply; 4+ messages in thread
From: hrn @ 2024-01-04 17:20 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 67681

Hi,

Thanks  for reviewing!

> Why is the test suite disabled?  A comment (explaining a good reason) 
> is
> needed :-).

This is what many node packages in node-xyz.scm does. I think this is 
because the tests are difficult to bootstrap. The dependency chains for 
the tests
are typically large and circular.

> A comment explaining why these are removed would help.

This is also the norm in node-xyz.scm. The excluded dependencies are for 
development and can in many cases be very difficult to bootstrap.

To be able to make progress with packaging node packages it seems to me 
that the general strategy has been  to disable tests and dependencies 
needed for tests. My assumption might be wrong though. Do you still 
think I should add comments?

> Could you please expand a bit on the description?  What features does 
> it
> have?  Keep it factual; "The complete solution ..." sounds like
> marketing language.

I agree that this needs to be changed. I blindly copied what the package 
said about itself.

Best regards,
Rikard




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

* [bug#67681] [PATCH] gnu: Add node-commander.
  2024-01-04 17:20   ` hrn
@ 2024-01-06 19:05     ` Maxim Cournoyer
  0 siblings, 0 replies; 4+ messages in thread
From: Maxim Cournoyer @ 2024-01-06 19:05 UTC (permalink / raw)
  To: hrn; +Cc: 67681

Hi,

hrn <hrn@posteo.net> writes:

> Hi,
>
> Thanks  for reviewing!
>
>> Why is the test suite disabled?  A comment (explaining a good
>> reason) is
>> needed :-).
>
> This is what many node packages in node-xyz.scm does. I think this is
> because the tests are difficult to bootstrap. The dependency chains
> for the tests
> are typically large and circular.
>
>> A comment explaining why these are removed would help.
>
> This is also the norm in node-xyz.scm. The excluded dependencies are
> for development and can in many cases be very difficult to bootstrap.
>
> To be able to make progress with packaging node packages it seems to
> me that the general strategy has been  to disable tests and
> dependencies needed for tests. My assumption might be wrong though. Do
> you still think I should add comments?

OK, this makes sense, but hard to grasp for a newcomer like me to (gnu
packages node-xyz).  Perhaps it could be explained in a commentary
comment at the top of the module.

>> Could you please expand a bit on the description?  What features
>> does it
>> have?  Keep it factual; "The complete solution ..." sounds like
>> marketing language.
>
> I agree that this needs to be changed. I blindly copied what the
> package said about itself.

OK.  I look forward to a v2.

-- 
Thanks,
Maxim




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

end of thread, other threads:[~2024-01-06 19:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-07  8:29 [bug#67681] [PATCH] gnu: Add node-commander Rikard Nordgren
2024-01-04  3:31 ` Maxim Cournoyer
2024-01-04 17:20   ` hrn
2024-01-06 19:05     ` Maxim Cournoyer

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.