On Sat, Aug 05, 2023 at 09:50:57PM -0400, Thomas Frohwein wrote:
> On Sat, Aug 05, 2023 at 11:27:24PM +0200, Marc Espie wrote:
> > Some comments already. I haven't looked very closely.
>
> > On Sat, Aug 05, 2023 at 03:12:18PM -0400, Thomas Frohwein wrote:
> > > The current draft hijacks post-extract target, but it would be easy to
> > > add this to _post-extract-finalize in bsd.port.mk similar to how the
> > > post-extract commands from modules are handled, if this is of interest.
> >
> > Please do that.
>
> Thanks, I updated the draft. Realized that including it with
> MODULES=github is easiest and then this can just use
> MODGITHUB_post-extract and doesn't need any custom code in bsd.port.mk.
> I had a thinko in post-extract (needs to be '||', not '&&') which is
> also corrected.
>
> > > # where submodule distfiles will be stored
> > > GHSM_DIST_SUBDIR ?= gh-submodules
> >
> > Please keep to the GH_* subspace.
> >
> > Already, modules usually use MOD* variable names, but in the GH case, that
> > would be a bit long.
>
> I renamed GHSM_* to GH_*. I wouldn't mind using MODGH_* if that's an
> option, but MODGITHUB_* would be pretty unwieldy, especially if we were
> to make the existing GH_{ACCOUNT,PROJECT,TAGNAME} etc. part of this.
>
> [...]
>
> > Please do a single loop. That's slightly more readable for me.
>
> yes, done.
>
> [...]
>
> > Also please draft a diff for port-modules(5)
>
> I'm attaching a diff for port-modules.5, along with the updated
> github.port.mk.
>
> > I'm also wondering if keeping the main GH_* stuff in bsd.port.mk makes a lot
> > of sense, instead of grouping everything in github.port.mk
>
> I'm for it, maybe as a second step after the module for just the
> submodule handling is done because there would be a lot of ports churn
> with moving the main GH_* stuff out of bsd.port.mk.
Probably not. We have a few "big" modules that don't depend on explicitly
adding to modules, but on some variable triggering it (historic imake stuff
or configure), having github.port.mk brought in from one of the GH_* variables
(probably don't need to test them all) would be acceptable.
