Re: git: 4ca613c4414b - main - irc/eggdrop: Fix build, bump PORTREVISION

On Tue, Feb 04, 2025 at 04:10:50PM +0000, Ryan Steinmetz wrote:
> On (01/30/25 05:57), Alexey Dokuchaev wrote:
> > On Wed, Jan 29, 2025 at 06:14:30PM +0000, Ryan Steinmetz wrote:
> > > commit 4ca613c4414b2dfaa9de5336b6da5a74aa73f5d4
> > > 
> > >   irc/eggdrop: Fix build, bump PORTREVISION
> > > 
> > >   PR:             283584
> > > ...
> > >  post-patch:
> > > -	@${REINPLACE_CMD} -e 's|tcl.h|tcl${TCL_VER}/tcl.h|g' ${WRKSRC}/src/mod/python.mod/pycmds.c
> > > +	@${SED} -i '' -e '25d' ${WRKSRC}/src/mod/python.mod/pycmds.c
> > 
> > There are several issues with this commit.  It doesn't explain what
> > was the problem and how removing 25th line fixes it.  Wasn't there
> > a better anchor than the line number?  Also, why use ${SED} -i ''
> > instead of canonical ${REINPLACE_CMD}?  Last, if it's a *build* fix,
> > why bump port revision?
> 
> The submitter was following the exiting patterns (use of SED) in the port.
> 
> I've updated SED->REINPLACE_CMD and moved the line 25 deletion to a patch
> as I expect it will be replaced whenever upstream merges the open PR to
> address this.

Yeah, I've seen the commit, thank you.  Not that I really insisted on the
patch (see below), it was more of a general observation that we (all)
should review/rethink submitted work better prior to commit, and try to
write more elaborate commit logs because not everyone reading them would
be on the same page with committer or maintainer and have immediate access
to the PR or source tarball.

> From the PR:
> 
> In file included from .././python.mod/python.c:46:
> .././python.mod/pycmds.c:25:10: fatal error: 'tcl8.6/tcl.h' file not found
>    25 | #include <tcl8.6/tcl.h>
>       |          ^~~~~~~~~~~~~~
> 1 error generated.

It wasn't hard to figure, but this requires actually running "make patch".
E.g. /^#include <tcl8\.6/tcl\.h>/d is more descriptive than 25d and less
likely do delete something unintended if line numbers change; but oh well,
the patch would also do.

./danfe