{openresty,nginxModules.brotli}: updates and improvements #77804
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofborg
bot
requested review from
lblasc,
fpletz,
globin,
7c6f434c and
thoughtpolice
emilazy
force-pushed
the
openresty-module-support
branch
from
32dc395
to
885c3ea
Compare
|
It turns out that naming a parameter Let's try that again... @GrahamcOfBorg build nginx openresty |
|
Sorry for the delay reviewing this @emilazy -- yes, these changes look pretty good! Please rebase (it looks like there's just been a trivial update to |
thoughtpolice
approved these changes
Feb 4, 2020
The fork was merged back upstream but has yet to see a formal release.
The primary motivation of this change was to allow third-party modules to be used with OpenResty, but it also results in a significant reduction of code duplication.
emilazy
force-pushed
the
openresty-module-support
branch
from
885c3ea
to
a989fe2
Compare
|
should be good now :) @GrahamcOfBorg build nginx openresty |
lblasc
approved these changes
Feb 4, 2020
thoughtpolice
approved these changes
Feb 5, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This updates the
ngx_brotlimodule to the latest upstream version, and also rebases theopenrestypackage on top of thenginx/generic.nixinfrastructure, simplifying things considerably and allowing the use of third-party modules with OpenResty.I haven't tested this extensively yet, but the build completes successfully for me and the OpenResty codebase is just a distribution of nginx, so I think it should be pretty low-risk.
nix build '(with (import ./. {}); openresty.override { modules = with nginxModules; [ brotli ]; })'builds successfully with these changes.cc @thoughtpolice does this look reasonable to you?
@GrahamcOfBorg build nginx openresty
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)