GNU bug report logs - #74174
[PATCH] gnu: Add emacs-cargo-el

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74174 in the body.
You can then email your comments to 74174 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Javier Olaechea <pirata <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add emacs-cargo-el
Date: Sat, 2 Nov 2024 05:42:32 +0000

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

Hi,
attached is patch to add the cargo.el package. It is a minor mode to
interact with rust's cargo package manager from Emacs.

Cheers

-- 
"I object to doing things that computers can do." — Olin Shivers

[Message part 2 (text/html, inline)]

[0001-gnu-Add-emacs-cargo-el.patch (text/x-patch, attachment)]

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

From: Cayetano Santos <csantosb <at> inventati.org>
To: control <at> debbugs.gnu.org,74174 <at> debbugs.gnu.org
Subject: QA review for 74174
Date: Mon, 04 Nov 2024 14:11:12 +0100

user guix
usertag 74174 + waiting-on-contributor
thanks

Guix QA review form submission:

- sentences in description should be followed by two spaces
- check propagated-inputs,  emacs-rust-mode instead of
  emacs-markdown-mode ?
- first revision should be 0 instead of 1 ?

Items marked as checked: Commit messages, Package builds, Lint warnings

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

From: Javier Olaechea <pirata <at> gmail.com>
To: 74174 <at> debbugs.gnu.org
Date: Tue, 5 Nov 2024 04:54:49 +0000

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

Guix QA review form submission:

> - sentences in description should be followed by two spaces

Updated the description accordingly

> - check propagated-inputs, emacs-rust-mode instead of
> emacs-markdown-mode ?

No, the package doesn't depend on rust-mode at all. It shells out the cargo
and rustc instead. The require calls in the elisp files are

(require 'xref)
(require 'compile)
(require 'button)
(require 'markdown-mode)
(require 'tramp)

Of those, only emacs-markdown-mode is not a built-in package. Although we
could add xref and tramp as inputs. They are also distributed as separate
packages in GNU Elpa. This means they have a separate release cadence from
emacs. What do you think?

> - first revision should be 0 instead of 1 ?

Thanks, it is true 'hackers count from 0'. ^_^. Updated.

Cheers
-- 
"I object to doing things that computers can do." — Olin Shivers

[Message part 2 (text/html, inline)]

[v2-0001-gnu-Add-emacs-cargo-el.patch (text/x-patch, attachment)]

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

From: Cayetano Santos <csantosb <at> inventati.org>
To: pirata <at> gmail.com
Cc: 74174 <at> debbugs.gnu.org,
 GNU bug tracker automated control server <control <at> debbugs.gnu.org>
Subject: Re: [bug#74174]
Date: Tue, 05 Nov 2024 10:08:50 +0100

user guix
usertag 74174 + reviewed-looks-good
thanks

> Of those, only emacs-markdown-mode is not a built-in package. Although we
> could add xref and tramp as inputs. They are also distributed as separate
> packages in GNU Elpa. This means they have a separate release cadence from
> emacs. What do you think?

In my opinion, if the dependency is built-in with current emacs, there
is no need to pull anything else.

v1 looks good to me,

C.

Message #21 received at 74174-done <at> debbugs.gnu.org (full text, mbox):

From: Hilton Chain <hako <at> ultrarare.space>
To: Javier Olaechea <pirata <at> gmail.com>
Cc: 74174-done <at> debbugs.gnu.org, Cayetano Santos <csantosb <at> inventati.org>
Subject: Re: [bug#74174] [PATCH v2] gnu: Add emacs-cargo-el.
Date: Thu, 07 Nov 2024 09:25:10 +0800

Hi Javier,

On Tue, 05 Nov 2024 12:54:49 +0800,
Javier Olaechea wrote:
>
> [1  <multipart/alternative (7bit)>]
> [1.1  <text/plain; UTF-8 (quoted-printable)>]
> [1.2  <text/html; UTF-8 (quoted-printable)>]
> Guix QA review form submission:
>
> > - sentences in description should be followed by two spaces
>
> Updated the description accordingly
>
> > - check propagated-inputs, emacs-rust-mode instead of
> > emacs-markdown-mode ?
>
> No, the package doesn't depend on rust-mode at all. It shells out the cargo and rustc instead. The require
> calls in the elisp files are


We generally patch these references to paths within /gnu/store.


> (require 'xref)
> (require 'compile)
> (require 'button)
> (require 'markdown-mode)
> (require 'tramp)
>
> Of those, only emacs-markdown-mode is not a built-in package. Although we could add xref and tramp as inputs.
> They are also distributed as separate packages in GNU Elpa. This means they have a separate release cadence
> from emacs. What do you think?
>
> > - first revision should be 0 instead of 1 ?
>
> Thanks, it is true 'hackers count from 0'. ^_^. Updated.


There's no strict requirement on revisions :)
Just make sure (version + revision) is increasing when updating package source.


> Cheers
> --
> "I object to doing things that computers can do." ― Olin Shivers
> [2 v2-0001-gnu-Add-emacs-cargo-el.patch <text/x-patch; US-ASCII (base64)>]
> From 95bb683ce62fd38c9bbbc4efa4f3499d0cc4f647 Mon Sep 17 00:00:00 2001
> Message-ID: <95bb683ce62fd38c9bbbc4efa4f3499d0cc4f647.1730781951.git.pirata <at> gmail.com>
> From: Javier Olaechea <pirata <at> gmail.com>
> Date: Sat, 2 Nov 2024 00:36:57 -0500
> Subject: [PATCH v2] gnu: Add emacs-cargo-el


Don't forget to end the subject with ‘.’


> * gnu/packages/emacs-xyz.scm (emacs-cargo-el): New variable.
>
> Change-Id: I73a99eeb818fb1c7ab87cc15c5953beba818cb94
> ---
>  gnu/packages/emacs-xyz.scm | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index 59c804066a..fc8a5839b4 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -29667,6 +29667,29 @@ (define-public emacs-rustic
>      (license (list license:expat
>                     license:asl2.0))))
>
> +(define-public emacs-cargo-el
> +  (let ((commit "7f8466063381eed05d4e222ce822b1dd44e3bf17")
> +        (revision "0"))
> +    (package
> +      (name "emacs-cargo-el")
> +      (version "0.4.0")


The version 0.4.0 doesn't match upstream commit 7f8466063381, so we shouldn't
use it here.  Please see package guidelines on version numbers[1] for details.


> +      (source
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               (url "https://github.com/kwrooijen/cargo.el")
> +               (commit commit)))


Don't forget to add a file name to this origin, you can use ‘guix lint’ to
identify such issue.


> +         (sha256
> +          (base32 "1hvxdmyppvx04jyn07dnynlgbwyasv22k8dd4qa68mrj8i9mz484"))))
> +      (build-system emacs-build-system)
> +      (propagated-inputs (list emacs-markdown-mode))
> +      (home-page "https://github.com/kwrooijen/cargo.el")
> +      (synopsis "Emacs Minor Mode for Cargo, Rust's Package Manager")


Uppercase only when it's meaningful.


> +      (description
> +       "Cargo mode for Emacs.  This package gives you a set of key combinations to
> +perform Cargo tasks within your Rust projects.")


Use full sentences for description, "Cargo mode for Emacs" should be reworded.
Guidelines for synopses and descriptions are available in [2].


> +      (license license:gpl3+))))
> +
>  (define-public emacs-ztree
>    ;; Upstream provides no tag, but the commit below matches latest release.
>    (let ((commit "c9ad9136d52ca5a81475693864e255d29448f43f"))
>
> base-commit: 33665c52c4670bc3b4d337c89ac9cc6c4c69b26f
> --
> 2.46.0


I have adjusted your patch based on above comments and applied it as
673b924ac1e30a5d498e28859af365cf2bb4a508, thanks!

Also thanks for Cayetano's previous review!

---
[1]: https://guix.gnu.org/manual/devel/en/html_node/Version-Numbers.html
[2]: https://guix.gnu.org/manual/devel/en/html_node/Synopses-and-Descriptions.html

Previous Next

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