Page MenuHomeFreeBSD

irc/inspircd: Update to 2.0.24; Add GEOIP option
ClosedPublic

Authored by yuri on Nov 8 2017, 2:31 AM.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This Makefile could really use OPTIONS_SUB and some sorting.

Also, you should probably add @feld as a reviewer.

Added OPTIONS_SUB.
Sorted options.

Reviewers: tcberner, adamw, feld

irc/inspircd/Makefile
1 ↗(On Diff #34923)

The created by lines are write only, unless the person in there asked you to remove the line, don't.

48 ↗(On Diff #34923)

I think the only change here is adding GEOIP, but as you moved everything, it is hard to see.

64–67 ↗(On Diff #34923)

It would probably be a good idea to change all those to use helpers.

GEOIP_​LIB_DEPENDS= libGeoIP.so:net/GeoIP
GEOIP_VARS=      ​  EXTRAS+=m_geoip.cpp
118 ↗(On Diff #34923)

And then change ${EXTRAS} to ${EXTRAS:S/ /,/g}.

yuri marked 2 inline comments as done.Nov 8 2017, 6:06 PM
yuri added inline comments.
irc/inspircd/Makefile
48 ↗(On Diff #34923)

adam@ asked to sort them.

Should I remove sorting?

yuri marked an inline comment as done.Nov 8 2017, 6:06 PM
yuri marked an inline comment as done.Nov 12 2017, 9:06 PM

@tcberner

Should I restore the original sortig and just keep the new option in?
This is a preexisting port. Only the addition of a new option was requested by the original poster, which is a very simple patch.
I would tend to not mix it with general cleanup, as this can inadvertently break things.

In D12990#271620, @yuri wrote:

@tcberner

Should I restore the original sortig and just keep the new option in?
This is a preexisting port. Only the addition of a new option was requested by the original poster, which is a very simple patch.
I would tend to not mix it with general cleanup, as this can inadvertently break things.

I would keep it sorted, as it is just better for maintenance. You might get some complaints for doing so, but that's a risk worth taking :D
[That's also why I ask you to have it sorted on new ports, because sorting it afterwards is always a hassle].

I agree, that it's generally a good idea to keep commits to "one thing". So you could create a diff for "option modernizing" afterwards.

Updated to 2.0.25.

Now only updating version and adding the GEOIP option.

Let's commit this way first, and then add sortedness, and implement mat's suggestions about helpers, etc?

yuri marked 2 inline comments as done.Nov 18 2017, 7:42 PM

Here GEOIP option is added, as requested by the original submitter. Once committed, I will re-sort options.

This revision was automatically updated to reflect the committed changes.