[Bug 203204] [NEW PORT] databases/pgmodeler: PostgreSQL Database Modeler

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=203204

Raphael Kubo da Costa <rakuco at FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rakuco at FreeBSD.org

--- Comment #2 from Raphael Kubo da Costa <rakuco at FreeBSD.org> ---
I've taken a look at the new port and have some comments and suggestions:

* You should set USE_GITHUB to "yes" instead of just defining it. Just defining
it also works, but it's not the common idiom used in the tree.

* You don't need to set GH_PROJECT if it has the same value as PORTNAME.

* You also don't need to set GH_TAGNAME: there's an actual 0.8.1 release on
GitHub, so just setting PORTVERSION and DISTVERSIONPREFIX works just fine.
You'll need to re-run make makesum though as the file name will change.

* Once you do that, setting WRKSRC is also not necessary.

* qt5-dbus and qt5-imageformats do not seem to be actual dependencies after
taking a look at the code.

* Including bsd.port.{pre,post}.mk just because you are setting some targets is
not necessary. Just including bsd.port.mk at the end works fine.

* Now speaking of those targets and your patches in files/:
  - Most of those patches become unnecessary if you use USES=qmake instead of
USES=qmake:outsource. Unfortunately the upstream code seems to assume you are
not doing an out-of-tree build and references things like $$PWD in the .pro
files. Instead of doing a lot of patching, it is much easier to just not do an
out-of-tree build until upstream fixes this.
  - I don't see why you are enabling the installation of the plugins. It
requires additional patching and upstream seems to have made a conscious choice
of only building tests and samples in a debug build. You should also maybe add
a DEBUG option and handle those additional plugin files in pkg-plist with the
%%DEBUG%% variable.
  - Personally, I don't like performing sed calls like the ones you do in the
post-patch target. Having actual patch files makes it easier for other people
to see what is being changed and why.
  - When patching main.pro and main-cli.pro, you could use
$$QMAKE_LIBS_EXECINFO instead of -lexecinfo directly.

-- 
You are receiving this mail because:
You are the assignee for the bug.