Saturday, December 04, 2021

Re: Feedback on upcoming port [playerctl]

Klemens Nanni wrote:
> On Sat, Nov 06, 2021 at 07:12:23PM +0000, Klemens Nanni wrote:
> > On Sat, Nov 06, 2021 at 07:40:33PM +0000, Lewis ingraham wrote:
> > > Thank you for answering! So I have done what you recommended. Would this
> > > revision suffice?
> >
> > Better but still lack tabs and newlines, comments can be zapped.
> >
> > More importantly, use meson OR cmake, not both. The project only uses
> > the former, so remove the cmake module.
> >
> > Shared libraries are best annotated with their original versions as per
> > ${WRKBUILD}/shared_libs.log.
> >
> > audio is the wrong category, imho. Players do video as well, so I'd go
> > with multimedia.
> >
> > Your port builds and packages playerctld which could benefit from a
> > proper playerctld.rc script so users can do use it via rcctl(8).
> > Not a requirement and I have not (yet) used playerctld, but certainly
> > worth thinking about.
> >
> > The rest looks good, the targets `port-lib-depends-check' and
> > `update-list' show no missing pieces and playerctl(1) just works with
> > ncspot on amd64 for me, nice!
> >
> > With meson, SEPARATE_BUILD=Yes is the default already.
> >
> > I've done all of the above for you, here's an updated tarball.
> > OK kn for someone to import this -- I can also import on your behalf
> > with another OK.
>
> Now that I know of the existence of playerctl(1), I'd be quite happy to
> have it in ports.
>
> Did anyone look at this? Lewis, do you have further input?
> I've reattached my updated version of your work, thanks again.

The usual checks pass. Plist looks correct. I tested it shortly with vlc
on amd64. This is quite an interesting port.

Nit: One of the tabs after GH_ACCOUNT is spaces.

ok sdk@

No comments:

Post a Comment