Subject: Re: [bug#62640] [PATCH v3] gnu: Add pmbootstrap.

From: André Batista <[email-masked]>
Date: Sat, 24 May 2025 13:26:17 -0300

merge 62640 49577
quit

sáb 24 mai 2025 às 14:59:48 (1748109588), [email-masked] enviou:
> (...)
> 
> > > +       (sha256
> > > +        (base32 "16naj8ykipbrs2a93zaxci1wl045cc9jn7rq8sy76d1rqlqb9mmw"))))
> > > +    (build-system pyproject-build-system)
> > > +    (arguments
> > > +     (list #:test-flags
> > > +           ;; Tests require a pmaports git repository in the workdir.
> > > +           #~(list (string-append "--deselect=test/core/test_pkgrepo.py"
> > > +                                  "::test_pkgrepo_pmaports")
> > 
> > This is a nitpick of mine, but I find it confusing when keys starting
> > with '#' are followed by their arguments on another aligned line which
> > also starts with '#' (a gexp this time, not another key). So, I'd suggest
> > indenting the arguments with two spaces to ease reading.
> 
> Like this?
> 
> (arguments
>  (list #:test-flags
>          ;; Tests require a pmaports git repository in the workdir.
>          #~(list (string-append "--deselect=test/core/test_pkgrepo.py"
>                                 ...))))
> 

Precisely.

> I would rather do it like this:
> 
> (arguments
>  (list
>    ;; The first two tests require a pmaports git repository in the workdir.
>    #:test-flags #~(list (string-append "--deselect=test/core/test_pkgrepo"
>                                        ...))))
> 

This would be even better, IMO.  But after reviewing some files, I think
this is just my personal take on it, not a guix standard.  So your patch,
your take.

> > Also, I would also have inserted this package directly on android.scm,
> > since this tool is useful for android related activities and it seems
> > overkill to create a new file for just one package.
> 
> Besides handling the Android bootimg format, I don't think pmbootstrap
> is Android related.  Yes, the only other major PostmarketOS project is
> pmaports, but I don't think there is an existing module where
> pmbootstrap belongs.  Should it be a more general module like: (gnu
> packages smartphone)?
>

Makes sense.  The only time I've used pmbootstrap was for analysing
android images, so I've given this functionality undue primacy.  I guess
we may keep it as is for now and move it elsewhere on a separate commit
if others see fit.
 
> Can you please close or merge issue #49577, because the PyPi pmbootstrap
> releases are apparently deprecated?
> 

You may also do it by sending a mail to [email-masked], as
I've done here.  There's no special priviledge to it.  See:

https://debbugs.gnu.org/server-control.html

Note: I'm not a commiter, I'm just trying to help this patch get through
by helping commiters with the review and giving it more visibility.

Thanks!