Page MenuHomeFreeBSD

irc/kvirc: update to 5.2.4
ClosedPublic

Authored by vvd on Tue, Apr 30, 12:28 PM.
Tags
Referenced Files
Unknown Object (File)
Sat, May 4, 6:34 PM
Unknown Object (File)
Fri, May 3, 9:26 PM
Unknown Object (File)
Fri, May 3, 6:50 PM
Unknown Object (File)
Thu, May 2, 9:21 AM
Unknown Object (File)
Wed, May 1, 8:24 PM
Unknown Object (File)
Wed, May 1, 1:05 AM
Unknown Object (File)
Wed, May 1, 1:03 AM
Unknown Object (File)
Wed, May 1, 1:03 AM

Diff Detail

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

Event Timeline

vvd requested review of this revision.Tue, Apr 30, 12:28 PM
vvd created this revision.
arrowd added inline comments.
irc/kvirc/Makefile
5–7

While there, can you switch WWW to https?

63

What does that mean?

81

No need for +=

85

No need to list duplicated deps.

irc/kvirc/Makefile
5–7

Ok.

63

Not linked with libgsm.so, but uses dlopen(libgsm.so) - stage-qa write warning about libgsm.so isn't used.

81

Copy&paste typo. Will fix it.

85

Used several libs from one port - I left all.
OK: "There can be only one"™

vvd marked 3 inline comments as done.
irc/kvirc/Makefile
63

Then it should be RUN_DEPENDS=${LOCALBASE}/lib/libgsm.so, I'd say.

irc/kvirc/Makefile
63

Really?! Didn't know. :-o

I have almost same for net/freerdp{,3}:

# LoadLibraryA("libpcsclite.so")
PCSC_LIB_DEPENDS=       libpcsclite.so:devel/pcsc-lite
PCSC_CMAKE_BOOL=        WITH_PCSC

Does this need to be fixed too?

irc/kvirc/Makefile
63

With the upcoming "autodeps" switch by bapt - yes, this is the right thing to do.

irc/kvirc/Makefile
63

And audio/audacity with option FFMPEG.

vvd marked 3 inline comments as done.

Upstream committed to master branch all fixes based on our patches - use this commit instead of local patches.

irc/kvirc/pkg-plist
3

The initial purpose of VER substitute is to prevent churn when doing updates. Adding a new substitution defeats its purpose. I'd leave %%VER%% as it is.

78

There are no other places where these substitutions are used, so I'm also against adding them.

irc/kvirc/pkg-plist
78

This substitutions help to update pkg-plist - don't forget to accidentally update the version when updating the port version. This is not such a rare mistake.
But if you, as a kde@, think otherwise, then I can only comply.

makc added inline comments.
irc/kvirc/Makefile
5–7

I'd drop "free portable" from the COMMENT. Overwhelming majority of the ports are "free" software, hence free is kind of expected. "Portable" — who cares about portability when you install it from ports/packages?

irc/kvirc/pkg-plist
3

I agree with Gleb, renaming VER to MINOR subverts the same reason VER was introduced for.

78

There's no need for MAJOR, it won't change for a long time.
Given we already have VER I'd use FULL_VER, VERSION or PORTVERSION instead of PATCH. See Qt/KDE and other ports for example.

vvd marked an inline comment as done.
vvd marked an inline comment as done.Fri, May 3, 7:10 PM
vvd added inline comments.
irc/kvirc/Makefile
5–7

Copy&paste from https://www.kvirc.net/:

KVIrc is a free portable IRC client based on the excellent Qt GUI toolkit.
irc/kvirc/pkg-plist
78

Returned VER, replaced PATCH with FULL_VER, removed MAJOR.

vvd marked an inline comment as done.Fri, May 3, 7:10 PM
vvd marked an inline comment as done.
vvd added inline comments.
irc/kvirc/pkg-plist
846

This is 2nd place with FULL_VER.

Vlad, there is no need in making option for everything. Instead, try to minimize list of options when creating ports. Group similar functionality under single option (or use RADIO/MULTI etc), don't make options for features that require light or common dependencies, especially if a feature does not need deps at all. Sometimes having too many options is worst than having no options.
And of course these are not 100% strict rules.

irc/kvirc/Makefile
36

Try to explain what DDC_VOICE/VIDEO means, e.g.
Direct client voice/video connection support

I'd also merged them into single option: is there a reason to have video "chat" without audio?

37

I'm surprised that ENCHANT is not in options.desc.mk yet.

38

This option does not need any dependency. I'd enabled corresponding cmake toggle by default and removed the option from the port.

Same for IPV6

39

KDE_DESC= KDE desktop environment support

It is not in options.desc.mk also! omg!

40

This description is quite generic and options.desc.mk is already provides it.

41

"Link preview via qtwebengine" or "Web browser plugin" depending on what this option enables.

46

qt5-dbus is already pulled by qt5-gui, thus this option really does not bring any new dependency. Again, I'd enabled dbus functionally by default and removed the option.

Small fixes in options.

vvd marked 2 inline comments as done.Sat, May 4, 3:42 PM
vvd added inline comments.
irc/kvirc/Makefile
36

It's 2 different independent features via DCC protocol:
/dcc.video NICK and /dcc.voice NICK

37

I can add it if you help with correct description.
Do I need approval from maybe portmgr@?

38

GTKSTYLE:

By default we disable QGtkStyle since it messes up with our theming
Some users want it anyway, and we sometimes need to check it

IPV6 option removed and added WANT_IPV6 to CMAKE_ON.

39

Same as for ENCHANT.

40
41

Description for WEBENGINE replaced with text from upstream:
https://github.com/kvirc/KVIrc/blob/5.2.2/CMakeLists.txt#L434
Don't know what it really do.

Old description for QTWEBKIT was: Use obsolete qt5-webkit to build Browser Plugin.
WEBENGINE replaced QTWEBKIT.

46

It linked with qt-dbus only if WANT_QTDBUS=true or WANT_KDE=true.
In other cases it doesn't linked with qt-dbus.

vvd marked 2 inline comments as done.Sat, May 4, 3:42 PM
vvd marked 4 inline comments as done.Sat, May 4, 10:16 PM
vvd retitled this revision from irc/kvirc: update to 5.2.2 to irc/kvirc: update to 5.2.4.
vvd edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Mon, May 13, 8:07 PM
In D45025#1028274, @vvd wrote:

Moin moin

I have no issue with moving this from kde@ to vvd@

mfg Tobias