mozilla / ssl-config-generator

Mozilla SSL Configuration Generator
https://ssl-config.mozilla.org/
Mozilla Public License 2.0
374 stars 60 forks source link

Fix stunnel outputs #258

Closed gstrauss closed 1 month ago

gstrauss commented 1 month ago

Stunnel refactor to correctly use the output data instead of hardcoding version conditionals, that never had the intended effect in the first place (where the template logic would only display modern config for all options, unless deliberately lowering the server versions).

Fixes #227

gstrauss commented 1 month ago

@gene1wood was the repo config changed recently?

Review required New changes require approval from someone other than the last pusher.

And yet, under the Reviewers sidebar on the right:

At least 0 approving reviews are required to merge this pull request.

gstrauss commented 1 month ago

This merge fixes the issue in #227 where only the Modern config was displayed.

@gene1wood: it looks like the recent changes you made to the repo config have had the desired effect to remove the requirement for Approvals before merge. Thanks!

janbrasna commented 1 month ago

I would still prefer testing and approval from someone else than the author, but this was so broken it's better to publish now, and refine later. (I will be looking into formatting and also going through the version logic as described by the OP in their contribution — to follow what should be in the hbs and what's already taken care of in js — on a first glance this looks right, but will give it more than a first glance, however the priority is lower than the other things. I'd still generally expect a say ~14day window for reviews before self-approvals unless something's a hotfix though. Not a problem here, great job getting this sorted out so promptly, thanks!)

@gene1wood This deploy at least confirms the publishing source settings are A–OK and we're good to publish just the built artifacts now, and not compete with the static deploy;) Thanks!

gstrauss commented 1 month ago

@gene1wood This deploy at least confirms the publishing source settings are A–OK and we're good to publish just the built artifacts now, and not compete with the static deploy;) Thanks!

@gene1wood would you like a separate issue opened and assigned to you to track these requests? Please disable the static deploy (and document what was done in case it needs to be re-enabled).

janbrasna commented 1 month ago

This deploy at least confirms the publishing source settings are A–OK and we're good to publish just the built artifacts

Please disable the static deploy

These were set correctly, and already working correctly, as can be confirmed from recent action runs. Thanks Gene, no action needed.

Screen Shot 2024-10-12 at 16 40 25

(2/2 is correct, instead of the 5/5 before…)