all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Vinicius Monego <monego@posteo.net>
To: Vivien <vivien@planete-kraus.eu>, 53402@debbugs.gnu.org
Subject: [bug#53402] Rebase it for the new python packages
Date: Thu, 03 Feb 2022 20:49:08 +0000	[thread overview]
Message-ID: <1161a7298bbbbe87ce48f4e3e4846021204ae1df.camel@posteo.net> (raw)
In-Reply-To: <df09d62f7aa0e6a21362f9b7df5a88ff0ddd0a41.camel@planete-kraus.eu>

Em qui, 2022-02-03 às 20:29 +0100, Vivien escreveu:
> Dear guix,
> 
> We have 2 new python packages, so my patch series creates a trivial
> conflict. Here is a new version that you can apply directly.
> 
> Best regards,
> 
> Vivien

Hi,

To avoid future merge conflicts, please move the packages somewhere in
the middle of the files instead of the bottom.

As a rule of thumb for Python packages with tests in Pytest, the check
phase is overriden and Pytest is called manually. When the tests are in
a subfolder inside the module, add a --pyargs <package> parameter to
the pytest command, see e.g. the python-cartopy package. I could run
the python-nibabel tests with this change without having to delete
anything.

If the tests still can't run because of missing data, it's fine to
source from the upstream repository instead of PyPI, or skip the few
tests that need them or at all if the repository doesn't ship a
setup.py. If tests are to be disabled, they should also have a comment
with the reason.

The 'test-less' packages shouldn't be needed AFAICS. Tests should run
by overriding the check phase as stated above (untested).

I also have a few comments about the patches in general:

> +    (source (origin
> +              (method url-fetch)
> +              (uri (pypi-uri "imageio-ffmpeg" version))
> +              (sha256
> +               (base32
> +               
> "0ff14079izsyxwf6ki68k9a7w5krjlal7lwqvzg2bbddl92l5spj"))))

Could you style it as

    (source
     (origin
       (method url-fetch)
       (uri (pypi-uri "imageio-ffmpeg" version))
       (sha256
        (base32
"0ff14079izsyxwf6ki68k9a7w5krjlal7lwqvzg2bbddl92l5spj"))))

and the other packages too?

Gexps should only be used when ungexp (#$) is used. On many patches
(e.g. python-nitime) ungexp is not being used.

When using gexp, it's better to style the arguments as:

+    (arguments
+     (list
+       #:phases
+       #~(modify-phases %standard-phases

to save columns (some of the packages exceeded the 78 columns limit),
instead of

> +    (arguments
> +     (list #:phases
> +           #~(modify-phases %standard-phases
> 

.

Some of the descriptions are not full sentences (e.g. in python-pytest-
harvest-minimal). Please check that descriptions are full sentences.

When sending an updated series, use patch versions with --reroll-
count=4 or -v4.

Could you send a v4 with the requested changes?

Vinicius





  reply	other threads:[~2022-02-03 21:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-20 22:46 [bug#53402] Add python-mne Vivien via Guix-patches via
     [not found] ` <handler.53402.B.164271885411910.ack@debbugs.gnu.org>
2022-01-21  8:17   ` [bug#53402] Add python-mne: fix or disable tests Vivien Kraus via Guix-patches via
2022-02-03 19:29 ` [bug#53402] Rebase it for the new python packages Vivien via Guix-patches via
2022-02-03 20:49   ` Vinicius Monego [this message]
2022-02-06 14:49     ` Vivien via Guix-patches via
2022-02-06 20:06       ` Vinicius Monego
2022-02-11  3:03         ` Vivien via Guix-patches via
2022-03-06 21:48           ` [bug#53402] Add python-mne Ludovic Courtès
2022-03-09 23:28             ` Vinicius Monego
2022-05-26 21:24   ` [bug#53402] Update dependencies and rebase on newer work Vivien Kraus via Guix-patches via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1161a7298bbbbe87ce48f4e3e4846021204ae1df.camel@posteo.net \
    --to=monego@posteo.net \
    --cc=53402@debbugs.gnu.org \
    --cc=vivien@planete-kraus.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.