Page MenuHomeFreeBSD

misc/sword: remove unnecessary use of REINPLACE_CMD
ClosedPublic

Authored by swills on Feb 7 2020, 1:49 PM.
Tags
None
Referenced Files
F83464666: D23568.id67935.diff
Fri, May 10, 7:54 PM
F83464659: D23568.id.diff
Fri, May 10, 7:54 PM
F83464121: D23568.id69892.diff
Fri, May 10, 7:50 PM
Unknown Object (File)
Mon, May 6, 4:11 PM
Unknown Object (File)
Mon, May 6, 12:02 PM
Unknown Object (File)
Sat, May 4, 2:59 AM
Unknown Object (File)
Wed, Apr 24, 4:55 PM
Unknown Object (File)
Sat, Apr 20, 4:58 PM

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I understand that the file you add does the same thing than the ${REINPLACE_CMD}, but why do you prefer using a file? I do not see the advantage. On the contrary I see disadvantages:

  • your file is more byte expensive than the ${REINPLACE_CMD} (sure, in 2020 it does not do much difference for most users, but we never know);
  • your file is harder to maintain: it might need to be recreated for any update of the port, while the ${REINPLACE_CMD} will most probably always work.

I understand that the file you add does the same thing than the ${REINPLACE_CMD}, but why do you prefer using a file? I do not see the advantage. On the contrary I see disadvantages:

  • your file is more byte expensive than the ${REINPLACE_CMD} (sure, in 2020 it does not do much difference for most users, but we never know);
  • your file is harder to maintain: it might need to be recreated for any update of the port, while the ${REINPLACE_CMD} will most probably always work.

REINPLACE_CMD can break silently; that is, it stops working, but nothing tells you that.

You're right.
Let's commit it then.

This revision is now accepted and ready to land.Feb 7 2020, 2:59 PM

It seems this review has been forgotten for a month. In the meantime I became a (mentored) committer: do you want me to commit it?

I added my mentors to the review. If they approve it, I will commit it (commit message identical to the title of this review).

Looks fine (assuming you have tested as usual).

Just a comment on the commit message: the use of REINPLACE_CMD was not unnecessary, you are changing it for a different approach, so perhaps "Replace use of REINPLACEMENT_CMD by a patch" or something along those lines?