On Thu, Jun 04, 2020 at 07:57:09PM -0500, Lucas Raab wrote:
> On Thu, Jun 04, 2020 at 03:18:39PM +0200, Landry Breuil wrote:
> > On Wed, Jun 03, 2020 at 08:12:48PM -0500, Lucas Raab wrote:
> > > On Wed, Jun 03, 2020 at 07:06:28AM -0500, Lucas Raab wrote:
> > > > On Wed, Jun 03, 2020 at 12:56:00PM +0100, Stuart Henderson wrote:
> > > > > On 2020/06/03 06:02, Lucas Raab wrote:
> > > > > > On Wed, Jun 03, 2020 at 08:19:40AM +0200, Landry Breuil wrote:
> > > > > > > On Tue, Jun 02, 2020 at 05:01:06PM -0500, Lucas Raab wrote:
> > > > > > > > Hello,
> > > > > > > > 
> > > > > > > > Here are three new ports, two deps, and the one piece de 
> > > > > > > > resistance,
> > > > > > > > web2ldap.
> > 
> > <snip>
> 
> * I added NO_TEST to py-ldap0. Looking at the repo, the tests look to
>   assume that they're running in the container in .gitlab-ci.yml
> * I added MODPY_PYTEST=Yes to py-xlwt, tests fine after that
> * I added MODPY_PYTEST=Yes to web2ldap, but for some reason, the tests
>   don't seem to be picked up. I'm not sure what to do there. Nothing
>   stood out to me as being obviously wrong.

thanks, that part looks good for py-xlwt. for web2ldap it seems the
tests/ subdir isnt shipped in the tarball, nor for py-ldap0. Maybe
they're only on github ? py-ldap0 only has ldap0/test.py but that's to
spawn a testing instance for the daemon. That often happens with python
ports which dont ship tests in releas tarballs, maybe via
'packages=find_packages(exclude=['tests']),' in setup.py ?

> > something i spotted - MODPY_BIN should be used in pkg/web2ldap.rc, dont
> > hardcode python3.7.
> 
> Updated, thanks for catching that.

another nit - i know some other py-ports with daemons also use
${daemon_flags:+ ${daemon_flags}} in the pexp in case someone sets some
daemon_flags via rcctl - might want to check if they are appended to the
process name/commandline and if the pexp still matches.

> > $doas ln -s /usr/local/share/examples/web2ldap/templates 
> > /etc/web2ldap/templates
> 
> I added back the @exec-add from sthen. Yeah, that was a bad move on my
> part by taking it out.

Thanks!

> > Adding BUILD_DEPENDS to RUN_DEPENDS is to be avoided, for example here it
> > installed devel/ccache for example..
> 
> Updated to move them to RUN_DEPENDS since none of them were actually
> required to build the package.

Thanks - well at that point it looks good to import to me. Anyone
wanting to have another look or import them with my ok ?

Landry

Reply via email to