Page MenuHomeFreeBSD

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

Authored by yuri on Nov 8 2017, 2:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 21, 10:43 PM
Unknown Object (File)
Mar 19 2024, 3:33 PM
Unknown Object (File)
Mar 11 2024, 11:12 AM
Unknown Object (File)
Mar 11 2024, 11:12 AM
Unknown Object (File)
Mar 11 2024, 11:12 AM
Unknown Object (File)
Mar 11 2024, 11:12 AM
Unknown Object (File)
Mar 11 2024, 11:12 AM
Unknown Object (File)
Mar 11 2024, 11:12 AM
Subscribers

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 12843
Build 13107: arc lint + arc unit

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

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

50

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

66–69

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
150

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
50

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.