Page MenuHomeFreeBSD

New port: audio/drumstick: MIDI libraries for Qt5/C++
ClosedPublic

Authored by yuri on Nov 8 2017, 5:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 4, 8:21 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Unknown Object (File)
Thu, May 2, 8:27 AM
Subscribers

Diff Detail

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

Event Timeline

audio/drumstick/Makefile
18 ↗(On Diff #34926)

^ buildtools_build qmake_build

51 ↗(On Diff #34926)

^ I would add patches for the CMakeLists.txt/*.cmake ones at least -- also looks upstreamable :)
[i.e. create a patch for upstream, and add it as a patch to files/ -- that way, you have to change one thing (remove the patch) once the fixed release appears, and not scratch your head about "do I still need this?" ^^]

59 ↗(On Diff #34926)

a nicer way is to patch in an OPTION DOCS to the CMakeLists.txt and add an if (WITH_DOCS) or just a simple if (BUILD_DOCS) around the add_subdirectory which you pass with DOCS_CMAKE_BOOL=BUILD_DOCS

64 ↗(On Diff #34926)

^ why not tell cmake to not install it?
if (ALSA_FOUND) ...

audio/drumstick/Makefile
31–34 ↗(On Diff #34926)

I know these kind of long sed lines are fun to write, but they are unmaintainable in the long term, it is better to patch the file with a real patch. (And then, you have a patch you can upstream.)
Only sed when you replace things with variables, like ${PREFIX} or such.

yuri marked 5 inline comments as done.Nov 9 2017, 6:39 AM
yuri added inline comments.
audio/drumstick/Makefile
31–34 ↗(On Diff #34926)

I left only those that require variables.

yuri marked 2 inline comments as done.Nov 9 2017, 6:39 AM

Thanks for converting this to patches :)

audio/drumstick/Makefile
27 ↗(On Diff #34976)

Looking at the files installed by the DOCS option, maybe MANPAGES would be more appropriate.

audio/drumstick/Makefile
20 ↗(On Diff #35074)

^ sorry, spotted that just now -- this is a Qt port

Macro this-is-freebsd: s/FreeBSD/Qt

yuri marked an inline comment as done.Nov 11 2017, 7:57 AM
This revision is now accepted and ready to land.Nov 11 2017, 7:58 AM
This revision was automatically updated to reflect the committed changes.
head/audio/drumstick/Makefile
22

MANPAGES should be added to OPTIONS_DEFAULT, this is FreeBSD, not Linux, we provide man pages by default.

yuri marked an inline comment as done.Nov 12 2017, 5:52 PM

MANPAGES added to OPTIONS_DEFAULT.

In D12999#271516, @yuri wrote:

MANPAGES added to OPTIONS_DEFAULT.

You need to bump PORTREVISION when doing it.

Tobias,

Sorry for this misunderstanding!

So, should I bump PORTREVISION now?

In D12999#271590, @yuri wrote:

Tobias,

Sorry for this misunderstanding!

So, should I bump PORTREVISION now?

No worries :)

PORTREVISION

PORTREVISION must be increased each time a change is made to the port that changes the generated package in any way. That includes changes that only affect a package built with non-default options.

by enabling MANPAGES you did exactly that.

So, yes, please give it a bump :)