On Fri, Aug 20, 2021 at 09:32:32AM +0200, Stefan Hagen wrote:
[...]
> > > [...]
> > > > > > This version is OK sthen@ to import.
> > > 
> > > It would seem to me that the hardcoded '/usr/local' in
> > > patches/patch-build_libs_mk is better replaced with ${LOCALBASE} and a
> > > ${SUBST_CMD} for the file.
> > > 
> > > I have made these adjustment (LOCALBASE use, SUBST_CMD, removal of the
> > > patch for cfg_bindings.lua, and addition of a pkg-readme) and attached
> > > an updated tarball with those.
> > > 
> > > ok thfr@ with or without the change to cfg_bindings.lua and the README;
> > > or I'd import it if there's an ok from another dev for these adjustments.
> > > 
> > > [1] https://github.com/raboof/notion/issues/336
> > 
> > Please pass LOCALBASE in as a make variable rather than using SUBST_CMD
> > in do-gen, and keep the comment about lua next to the pre-configure
> > target to which it refers.
> 
> Attached again with LOCALBASE passed via MAKE_FLAGS and the lua comment
> at the right spot again.
> 
> I'm ok with the pkg/README and leaving the problem for upstream. Notion
> needs a lot of keys. I'm also getting the popup about the grave key here
> on a German keyboard where only dead_grave exists per default.
> 
> However, having a look at the key bindings and doing some adjustments is
> probably the first step for everyone trying a new window manager.
> 
> I rewrote the README in a more general way and less targeted at
> Caps_Lock.

Looks good to me, still builds, way better without do-gen target, and I
like the README much better; only after variable expansion the line with
${LOCALBASE} will still exceed 80 chars length.

I'd be happy to import this with reflowing the line to stay within 80
chars length if there's another ok...

Reply via email to