search

tiredofit / docker-nginx-php-fpm

Dockerized web server and interpreter image with many customizable options
MIT License
138 stars 34 forks source link

Fix llng_auth_request config empty params #51

Closed radarsymphony closed 1 year ago

radarsymphony commented 1 year ago

Hi @tiredofit,

I noticed that you have the same code/error as discussed in nginx PR #17 in the php-fpm image. While the php-fpm image did rebuild as you said, the fix to the nginx image was then replaced by the same issue in the php-fpm one.

I've applied the same patch as before. However, given the nature of your layered approach with your nginx image as the base for the php-fpm one, does duplicating this code allow additional flexibility/features? Would the solution be to just remove that function/block here from the php-fpm image if it's functionality is already covered by the nginx image now? Basically, while the same tweak (\ --> ^) works, maybe this PR isn't the approach and maybe there's an opportunity to de-duplicate?

I look forward to your thoughts on this option and any context you can provide for the duplication of this function.

tiredofit commented 1 year ago

While they look similar, they are different - This one specifically targets any descendant images.

Not sure why this is coming up now - that grep command has been in use since at least 2019.

tiredofit commented 1 year ago

Tagged as 7.4.2