Created attachment 184717 [details] Shar archive of new port
Hello, thanks you for your contributions and on this port. I found a few items that needs to be adjusted before it is ready to be committed. * Instead of including the bsd.port.pre.mk with an if, this could be made to simply strip an i from the arch. ${ARCH:S/i//}. This should make it work with both i386 and amd64. * The USE_RC_SUBR belongs in the USES section and should come right after the ONLY_FOR_ARCHS. * Group similar variables together with a space for these items, EXTRACT_*(excluding EXTRACT_SUFX), NO_* and *_FILES. * The append flag on SUB_FILES is not needed.
Created attachment 185134 [details] Updated Makefile Better? ;) Thanks for your comments Patrick P.S. Can't test on i386 at the moment ...
(In reply to punkt.de Hosting Team from comment #2) Mostly however a couple more things after looking at a bit more. * The bsd.port.pre.mk can be removed and the *post.mk can be changed to bsd.port.mk. * The rc script is a bit out of standards. Can you please take a look at [1] and follow the example a bit more closely. Having a comment at the top with each user modifiable variable would be helpful. [1] https://www.freebsd.org/doc/en/books/porters-handbook/book.html#rc-scripts
Hi! Don't I need bsd.port.pre.mk for ${ARCH} to be defined? It says so here: https://www.freebsd.org/doc/en/books/porters-handbook/dads-after-port-mk.html I will look into the rc script in the meantime. Patrick
(In reply to punkt.de Hosting Team from comment #4) Thanks for pointing this out. I think this bit in PHB is outdated. I'm not positive on this though so leave it in for now and will have it checked.
A related note: the conditional statement for ${ARCH} in my initial version did not apply to the inclusion of bsd.port.pre.mk but is a separate step. 1. include bsd.port.pre.mk 2. act conditionally depending on the value of ${ARCH} And I still think my first version is easier to read and understand. ;-) But I can live with your version as well. Patrick
Please recreate the shar or make a diff of the entire port and obsolete the old files. A few more things to check: * Looking at the releases, arm should probably be added to ONLY_FOR_ARCHS list. * Instead of downloading from github with MASTER_SITES=, it would probably be better to use USE_GITHUB, and the GH_PROJECT/GH_ACCOUNT. * It looks like this just downloads the release. It is preferred to compile over downloading a release when possible.
So you'd rather add a dependency on Go than use the provided binary? I wouldn't ... I currently don't have the time to completely refactor this port to compile from sources. We use mailhog throughout the company for development, it is a great tool, and we create local ports and build packages in poudriere whenever possible. I just wanted to share ... Does the port as it stands have a chance to get committed or is installing the binaries a no-go? I would definitely improve the rc script in the next couple of days as requested. Thanks Patrick
(In reply to punkt.de Hosting Team from comment #8) As I said, compiling is preferred. It is completely understandable if you do not have the time and rather use the prepackaged release. It can be left as is in this case. If someone comes along and improves the port changing it to be compiling, that would be welcome. I don't think it is possible to add USE_GITHUB with prepackaged release. So this can be left as is. Fix the rc script, and it should be good. Thank you for contributing to the FreeBSD project.
Rc script improved, arm architecture added. Kind regards Patrick
Created attachment 185525 [details] Updated shar archive of entire port
(In reply to punkt.de Hosting Team from comment #11) Looks good, I found minor items, but will take care of them. Thank you for your contributions.
A commit references this bug: Author: ultima Date: Sun Aug 20 17:35:19 UTC 2017 New revision: 448436 URL: https://svnweb.freebsd.org/changeset/ports/448436 Log: MailHog is an email testing tool for developers: Configure your application to use MailHog for SMTP delivery View messages in the web UI, or retrieve them with the JSON API Optionally release messages to real SMTP servers for delivery WWW: https://github.com/mailhog/MailHog PR: 221015 Submitted by: punkt.de Hosting Team (maintainer) Reviewed by: matthew (mentor) Approved by: matthew (mentor) Differential Revision: https://reviews.freebsd.org/D12069 Changes: head/mail/Makefile head/mail/mailhog/ head/mail/mailhog/Makefile head/mail/mailhog/distinfo head/mail/mailhog/files/ head/mail/mailhog/files/mailhog.in head/mail/mailhog/files/pkg-message.in head/mail/mailhog/pkg-descr head/mail/mailhog/tags
Took some time and converted the port to being compiled over using pre-compiled package. Most of the port is the same except the package installs to bin instead of sbin. Committed, thanks!
A commit references this bug: Author: ultima Date: Sun Aug 20 18:30:17 UTC 2017 New revision: 448437 URL: https://svnweb.freebsd.org/changeset/ports/448437 Log: Removed tags file, this should never have been committed PR: 221015 Reviewed by: matthew (mentor) Approved by: matthew (mentor) Differential Revision: https://reviews.freebsd.org/D12069 Changes: head/mail/mailhog/tags