I see, i shouldn't have written "changed package". Like araujo stated, meta-data are not considered as part of the package. PORTREVISION bumps will rebuild the software from which the package is build. As long as the software contained (!) in the "package" does not change, such a bump is not needed.

In D22051#481655, @araujo wrote:

You don't need the gratuitous PORTREVISION bump here!

In D22000#480687, @dmgk wrote:

In D22000#480674, @tz wrote:

Also there is no need to separate the commits between updates and bumping the PORTREVISION. This can be done in one commit. So please commit this and D22001 together! :)

Thanks for approving. I was told before to split updates like this to two separate commits - https://reviews.freebsd.org/D21716#473777 :)

See my commit in D22000

Also there is no need to separate the commits between updates and bumping the PORTREVISION. This can be done in one commit. So please commit this and D22001 together! :)

I'm sorry, i missed your update. Its fine now, please go ahead and thank you! :)

In short: why? The commit message only state what the log says. But it does not tell us about the reasons. Why don't you want to install the test and the doc? I mean: normally this would be a sign of quality. Why not here?
I think the diff is fine, but the commit message needs to be improved

If possible please add the changelog or a link to the changelog of the update!

In D21819#476324, @araujo wrote:

In D21819#476317, @dmgk wrote:

Looks like svn ate "Changes:" line in the commit message, sorry about that. Is it some kind a reserved keyword? Should I use "Changelog" or similar for itemized changes list?

I felt strange after I doubled checked the svn log, I'm not aware about reserved keywords. Usually I use changelog, but we should have no limitations. @tz are you aware of any keywords or similar limitations?

Beside my comment, everything looks fine. Approved :)

In D21744#474777, @dmgk wrote:

In D21744#474762, @tz wrote:

Aloha Dmitri,
two short questions:

poudriere bulk -C -j120a: OK

Why only 12? Please always check both versions. Sometimes there are some side effects in building, nobody could foresee.

Hi Torsten,
Given that these ports are building fine now and there's no functional change to the software (no version updates etc) and only changes to the ports, I felt that it would be enough to test these patches only on a single FreeBSD version. I can run the same poudriere bulk builds on say 11.3 i386 and update test plan with results.

Aloha Dmitri,

In D21721#474028, @dmgk wrote:

In D21721#473929, @araujo wrote:

As you are working on this already for a while, how about ports that you are not a maintainer? Maybe an exp-run following with a portmgr@ approval would be interesting to clean up the rest of the ports.

That was my plan actually, I was just not sure what would be the best way to get something like that reviewed/committed. The complete diff is about 250KB, I was thinking about maybe breaking it to 3-4 parts for easier review. Would that make sense or it would be better to post it in a single review?

In D21349#473998, @joneum wrote:

There a wrong MAINTAINER lines, for example www/mod_php74:
MAINTAINER= ports@FreeBSD.org
I think, this Lines are needless?

In D21721#473887, @dmgk wrote:

In D21721#473870, @tz wrote:

Since i am very unfamiliar with the go ports, i have problems to understand this diff.
In devel/awless only do-build and do-install is removed, but i do not see any usage of GO_TARGET. Same for some other ports.

r505321 converted all ports to USES=go which already provides do-build and do-install targets. During that conversion, ports that were defining their own build/install targets were left alone to be cleaned up later. This commit does this cleanup for the ports that I maintain.

Is there any reason why removing the unneeded build-steps and the switch to GO_TARGET are in the same commit?

Removing targets doesn't remove any build steps, just switches the port to use default targets that go.mk provides.
GO_TARGET is used by go.mk in do-build and do-install, it doesn't make sense to set it with custom targets and removing custom targets w/o setting GO_TARGET would break the build.

Changed like requested by ale :)

Since i am very unfamiliar with the go ports, i have problems to understand this diff.
In devel/awless only do-build and do-install is removed, but i do not see any usage of GO_TARGET. Same for some other ports.

I'm sorry to bother you @ale , but without your approval i am not allowed to commit the needed change to php.mk. Can you please have a look at it?

This revision is from last year and i'm pretty sure you already fixed the issue @joneum , right?
In this case let us close the review.

Please notice: the hash module is gone in PHP 7.4. Its part of the core and can't be disabled anymore. You find details here:
https://reviews.freebsd.org/D21349

Approved!

Approved _=

Approved

Approved

Approved! Please go ahead a commit the news about you! :)

Add new module devel/php74-ffi and fix net-mgmt Makefile.

Fix another plist issue with the php-fpm config

Found and fix plist-issue for FPM

Thank you @tobik , this works! And it was the last piece. So everything is compiling and running now! :)

@tobik Thank you, this works with RC 1 too! :)

@ale Can you please review the changes of php.mk?
I would like to commit the RC1 without the broken modules imap and gd. I do not want to wait until the end and miss the interval to have runtime testing with an RC.

Update to RC 1.

You can find the diff with --copy-as-ads here:
http://www.meisterderspiele.de/freebsd/php-7.4.diff

I updated the review to contain the diff only, like requested. Hopefully you can work with this better now.

In D21349#464775, @ale wrote:

As you said, this is useful for testing, but a standard diff might be useful, too, to see what actually changed from php 7.2

Before you asked: yes, all is a repo-copy. I used --show-copy-as-ads in order to let you use the review for your own build-tests.