On Sat, 17 Apr 2021, jgart via Guix-patches via wrote:

> Hi Guix!
>
> Attached is a patch for sc-im, a terminal based spreadsheet program providing a vim-like experience.

Interesting program, thanks for working on the package

> I added Ekaitz as a co-author and added both of our copyrights to the top of the file.

Glad to see this work being picket up and that we can collaborate across 
time.

> This new version of sc-im is from 16 days ago.
>
> I'm currently getting the following two linter warnings:
>
> the source file name should contain the package name
> permanent redirect from https://github.com/andmarti1424/sc-im.git to https://github.com/andmarti1424/sc-im
>
> Any suggestions for what I need to adjust to make those pass?

I have some inline suggestions as follows:


From 62b2b692329f8db791db08700821111238ed40be Mon Sep 17 00:00:00 2001
From: jgart <jgart@dismail.de>
Date: Sat, 17 Apr 2021 17:43:16 -0400
Subject: [PATCH] gnu: Add sc-im.

     * gnu/packages/visidata.scm: New file.

Should be sc-im.scm there :)

I have no opinion on if this warrants its own file, but I expect others 
would be willing to comment.

     * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.

     Co-authored-by: Ekaitz Zarraga <ekaitz@elenq.tech>


+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2020 Ekaitz Zarraga <ekaitz@elenq.tech>
+;;; Copyright © 2021 jgart <jgart@dismail.de>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages sc-im)
+  #:use-module (guix packages)
+  #:use-module (guix git-download)
+  #:use-module (guix download)
+  #:use-module (guix build-system gnu)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (gnu packages)
+  #:use-module (gnu packages base)
+  #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages maths)
+  #:use-module (gnu packages statistics)
+  #:use-module (gnu packages xml)
+  #:use-module (gnu packages compression)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages ncurses))
+
+(define-public sc-im
+  (let ((commit-ref "75ae3806844821cba1b2e3fdb9237d737944e850"))
+    (package
+      (name "sc-im")
+      (version "0.8.1")
+      (source (origin
+                (method git-fetch)
+                (uri
+                  (git-reference
+                    (url "https://github.com/andmarti1424/sc-im.git")
+                    (commit commit-ref)))

You can remove the .git from the url to clear up the permanent redirect 
lint warning.

In other packages, we use the tag directly with something like

(commit (string-append "v" version))

and are thus able to forgo the let binding. Unless there is a reason to 
think upstream will move the tag to a different commit, I recommend doing 
that here too.

Also, adding `(file-name (git-file-name name version))` to the origin 
specification will clear up the file name lint warning.

+                (sha256
+                  (base32
+                    "1i1yq5mh9d7yi1bkgaq4p1lr8zrxhlvqmjnj33wmg5v6vpfim1h0"))))
+      (build-system gnu-build-system)
+      (arguments
+        ;; There are no tests at the moment.
+        ;; https://github.com/andmarti1424/sc-im/issues/537
+        ;; https://github.com/andmarti1424/sc-im/pull/385
+        `(#:tests? #f

Thanks for adding the comment about the lack of test.

+          #:make-flags (list "-C" "src" "CC=gcc"

We prefer `(string-append "CC=" ,(cc-for-target))` which helps when 
cross-compiling

+                         (string-append "prefix=" (assoc-ref %outputs "out")))
+          #:phases
+            (modify-phases
+               %standard-phases
+                 (delete 'configure))))
+      (inputs
+        `(("gnuplot" ,gnuplot)
+          ("libxls" ,libxls)
+          ("libxlsxwriter" ,libxlsxwriter)
+          ("libxml2" ,libxml2)
+          ("libzip" ,libzip)
+          ("ncurses" ,ncurses)))
+      (native-inputs
+        `(("pkg-config" ,pkg-config)
+          ("which" ,which)
+          ("bison" ,bison)))
+      (synopsis "Spreadsheet program with vim-like keybindings")
+      (description
+ "@code{sc-im} is a highly configurable spreadsheet program
+ providing a vim-like experience.  @code{sc-im} supports @{gnuplot} interaction,
+ functions for sorting and filtering, 256 color support, and much more.")
+      (home-page "https://github.com/andmarti1424/sc-im")
+      (license license:bsd-4))))
-- 
2.29.3


Can you send an updated patch?

Best,
Jack