D32369 landed in https://cgit.freebsd.org/ports/commit/devel/git?id=ff5ded75bcda28c580623504e01f5db8eb6be9cf

In D32369#749664, @bdrewery wrote:

Very nice. Thank you!

@garga I think this can be closed.

@garga Didn't find the time to test yet, therefore only suggestions to improve the UPDATING message.

In D30239#710881, @bdrewery wrote:

I've added a foot shooter in https://cgit.freebsd.org/ports/commit/?id=154492dd1f1d8c9a8f9c283f830a737c96e64a10

Thanks for the heads-up!

In D30238#697416, @grembo wrote:

@garga There is a MOVED entry on git-cvs, seems like there was something python based of that name before. So you would need to take care of this to allow the port to build in bulk (non-testport) mode.

@garga There is a MOVED entry on git-cvs, seems like there was something python based of that name before. So you would need to take care of this to allow the port to build in bulk (non-testport) mode.

In D30238#697016, @garga wrote:

@grembo Patch is not applying after latest changes on git port. Could you please rebase it? I finally found the time to work on it, sorry for the delay

Rebase to main

In D30807#695734, @ceri wrote:

'approved', 'allowed', 'rejected': may be clearer.

In D30807#695219, @kevans wrote:

So basically "blacklisted" was not the correct term to begin with.

This is my opinion, yes. I'm still searching for more ideal verbiage

@imp thanks for reviewing

In D30807#695022, @kevans wrote:

IMO this verbiage is all too active for what the feature really does. Trusted certs are trusted because we put them into a directory that OpenSSL understands to mean trusted, while these are simply certs that are not default-trusted.

Actually run mandoc -T lint, fix some more issues found.

@ceri I'm not sure if "untrusted" is a good choice in naming. To me, there were traditionally three levels:

@bdrewery But don't you think that

@mat @swills It doesn't seem like you got around to resolve this yet, so we're still in a situation where the documented way of using REINPLACE_ARGS will create different workdirs depending if DEVELOPER is on or not.

@lwhsu I see that the git repo was added to CGIs in the meantime (which is good), but the remainder of the navigation remains out of synch with what's on the main web site.

Just to repeat an earlier comment and asking portmgr to comment - then this review can progress or be abandoned and either way there will be a documented decision.

@ceri Thanks for taking the time to review.

@mat Next time, please show some basic respect and comment on a review that has been made obsolete by your own contribution (especially since I made it clear that I'm ok if you don't like it).

Make wording more general, use root prompt ("#").

In D30388#689933, @ceri wrote:

That sysctl is 2 by default, at least on -CURRENT.
Do we care that this is wrong on CURRENT (I don't think I do) ?

Adding docs, as this might be more appropriate(?)

In D30238#679780, @garga wrote:

I won't have the time to work on it until next week, but I like the idea

@adridg As you've been sending out regular messages about following the commit message conventions. What do you think?

In R11:f394c284629335d7828c5919f8b220e7d5a4640f#363606, @gerald wrote:

Umm, what's happening?

What kind of audit is required here? Why is an audit required?

Use contemporary terminology

In D30239#679284, @bdrewery wrote:

This isn't simply about host not matching the jail. It's more nuanced. The jail may not match the jail.

Does this change anything about the patch though (which in the end implements a force flag, which isn't nuanced at all)? Would you prefer a different name of the flag that emphasizes that you think this error shouldn't be ignored?

Updated the diff using arcanist, as using raw diff upload the way I did won't get us far.

Updating diff using arcanist

Put some more effort into this...

Minor changes. It's a bit ugly how options are defined and excluded right now,
but it works well enough for someone (hopefully) take it from there.

See also:

Sorry for all the noise, .arcconfig vs. git migration.

Add quoting like in the original template.

Add main branch to .arcconfig (we don't use master)

Prevent extra empty line when ammending.

ping?

In D28369#636198, @carlavilla wrote:

Sorry, I put the comment in spanish hehehehe

Thanks for the patch, but before making the commit to the repo we need to think how to deploy the cgi scripts in the new jail

You can reproduce the problem easily by going to:

In D28354#634399, @carlavilla wrote:

Approved. And commit done. Thanks for that.

@carlavilla This builds now locally, so it can be properly reviewed.

Fix things after incorporating changes that happened in the meantime.

I'll make it build locally first and then re-open...

@swills p.s. Besides the patch being simple, I figured that not calling sed_checked.sh was in line with what is done in case REINPLACE_CMD has been overridden, as changing REINPLACE_ARGS is also a way to alter REINPLACE_CMD.

In D27954#624840, @swills wrote:

In D27954#624793, @swills wrote:

It seems to me, and maybe I'm wrong, that this particular case could have been avoided by checking the output of make makeplist more carefully, it's not perfect, never has been and never will be. Am I missing something?