From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH v4] gnu: Add teensy-loader-cli.
Date: Sat, 04 Jun 2016 23:30:55 +0200
Message-ID: <87inxowsj4.fsf@gnu.org>
References: <1464863708-18392-1-git-send-email-h.goebel@crazy-compilers.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44511)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1b9J9t-0005K2-FN
	for guix-devel@gnu.org; Sat, 04 Jun 2016 17:31:06 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1b9J9n-0005J9-Ey
	for guix-devel@gnu.org; Sat, 04 Jun 2016 17:31:04 -0400
In-Reply-To: <1464863708-18392-1-git-send-email-h.goebel@crazy-compilers.com>
	(Hartmut Goebel's message of "Thu, 2 Jun 2016 12:35:07 +0200")
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel/>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: "Guix-devel" <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
To: Hartmut Goebel <h.goebel@crazy-compilers.com>
Cc: guix-devel@gnu.org

Hi!

Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> * gnu/packages/flashing-tools.scm (teensy-loader-cli): New variable.

I=E2=80=99m sorry for the extra round trip=E2=80=A6  Last one, I promise!  =
:-)

> +  (let* ((commit "f289b7a2e5627464044249f0e5742830e052e360")
> +         ;; Mind changing the version in the usage-message below when ch=
anging
> +         (version (string-append "2.1-1." (string-take commit 7))))

This =E2=80=98version=E2=80=99 variable is really unneeded, because:

  (package
    (name "foo")
    (version "bar")
    =E2=80=A6)

in fact expands to:

  (let ((name "foo") (version "bar") =E2=80=A6)
    (make-struct <package> name version =E2=80=A6))

This is done precisely so we can refer to field values.

So please remove the =E2=80=98version=E2=80=99 variable.  :-)

> +  (package

Add two columns to the indentation.

> +           ;; remove example flash files
> +           (for-each delete-file (find-files "." "\\.hex$"))
> +           ;; Remove teensy rebooter flash binaries
> +           (for-each delete-file (find-files "rebootor" "\\.(elf|hex)$"))

This is equivalent to:

  (find-files "." "\\.(elf|hex)")

because =E2=80=98find-files=E2=80=99 is recursive.

Also, please write sentences that start with a capital letter and finish
with a period.

> +           ;; fix the version
> +           (substitute* "teensy_loader_cli.c"
> +             (("Teensy Loader, Command Line, Version 2.0\\\\n")
> +              ;; TODO: find a way to pass the version from above
> +              (string-append "Teensy Loader, Command Line, Version 2.1-1=
\\n")))

Refer to =E2=80=98version=E2=80=99 here instead of hard-coding it:

  (snippet
    `(begin   ; <- note the backquote here, aka. =E2=80=9Cquasiquote=E2=80=
=9D
       =E2=80=A6
       (substitute* =E2=80=A6
         (string-append "foo" ,version "\n"))))

> +    (synopsis "Command line firmware uploader for teensy development boa=
rds")

s/teensy/Teensy/

> diff --git a/gnu/packages/patches/teensy-loader-cli-usage.patch b/gnu/pac=
kages/patches/teensy-loader-cli-usage.patch
> new file mode 100644
> index 0000000..097c61e

Please add an explanation and the upstream status to the beginning of
the patch.

Could you send one last version of the patch please?

Thank you, and apologies to taking so long and being so picky!  ;-)

Ludo=E2=80=99.