search

We-Amp / ngx-pagespeed-alpine

ngx_pagespeed docker image for Alpine. Moved over to https://github.com/apache/incubator-pagespeed-ngx/tree/master/docker
Apache License 2.0
35 stars 26 forks source link

rename mod-pagespeed to show stable #4

Closed webjay closed 6 years ago

webjay commented 7 years ago

Shouldn't mod-pagespeed-beta-1.12.34.3.tar.bz2 be renamed to mod-pagespeed-stable-1.12.34.3.tar.bz2 now that it uses the stable branch?

oschaaf commented 7 years ago

Nice catch! I agree

oschaaf commented 7 years ago

(In fact, 1.12.34.3 was nginx-only: https://www.modpagespeed.com/doc/release_notes#release_1.12.34.3-stable) So this should be 1.12.34.2-stable.

Fortunately, no code changes in PSOL have been made between any of those.

oschaaf commented 7 years ago

@ashishk-1 can you look into this?

webjay commented 7 years ago

I can ship a pr

oschaaf commented 7 years ago

@webjay that would be great! It would be best if @ashishk-1 reviews it though.

ashishk-1 commented 7 years ago

Looks good to me

oschaaf commented 7 years ago

Great! Can you upstream the proposed changes to this repo?

ashishk-1 commented 7 years ago

Done!

oschaaf commented 7 years ago

@ashishk-1 thanks.. Should we also push the dockerfile-36 branch over here?

ashishk-1 commented 7 years ago

@oschaaf we will need to tag 3.4 version first (which will need dockerfile update of pagespeed tarball url), then we can update the dockerfile with 3.6 version and tag it accordingly. wdyt?

oschaaf commented 7 years ago

@ashishk-1 It looks to me like it is desireable to include changes from https://github.com/ashishk-1/ngx-pagespeed-alpine/commit/99a074cb436241200345ab49ba4eb65c4911023b#diff-3254677a7917c6c01f55212f86c57fbf here, regardless of Alpine version? Generally, I'm wondering if it is desireable for this repo to lag behind. Regardless, we do need to update the docker file here before tagging 3.4 if I understand correctly, right?

Looking at the Docker file, I wonder if it would be a good idea to offer a site where you enter the desired ngx_pagespeed version, alpine version, and nginx version into a form, and we generate the docker from a template.

ashishk-1 commented 7 years ago

@oschaaf I think, if we tag with 3.4 then should it be the final thing for 3.4? But still we will need to update the pagespeed url in dockerfile with actual release tarball. If it is fine to retag(after updating url), or using some incremental version, it should be fine to tag it now also.

We can generate dockerfile by keeping ngx_pagespeed, alpine, nginx version in a form or in a separate input file. But this will need a solution that works on all platforms. Also there are(and can be in future versions) some changes in packages, flags from alpine 3.4 to 3.6. So dont know how convenient it is to compile all these changes together to generate dockerfile.

nberlee commented 6 years ago

This is all solved now