Maciej, Am 20.11.20 um 17:06 schrieb Maciej Zdeb: >> Merge the test into the previous commit, please. This ensures the test >> is always there when the feature is there and absent if it is not. >> > True! Fixed.
Okay, can you also add a reference to GitHub issue 909 to the commit message? Something like "This resolves GitHub issue #909" or "This is related to GitHub issue #909" is completely sufficient. Regarding your patch: > +#REQUIRE_VERSION=2.2 This should either be 2.4 or be removed. The removal should be safe, because it is integrated within a single commit. > +# This config tests various http request rules (set/replace) manipulating the > +# path, with or without the query-string. It also test path and pathq sample > +# fetches. I believe this comment is not correct. > + http-response del-header x-str2 -m str > + http-response del-header x-beg -m beg > + http-response del-header end1 -m end Indentation is messed up there, because you mixed tabs and spaces. > + * Otherwsize ACT_RET_ERR is returned. There is a typo there. > + rule->action = PAT_MATCH_STR; // exact matching (-m str) I know the single line comment style is sometimes used, but mostly it's always the multi line one. I suggest to change this to /* exact matching ... */ unless someone tells you otherwise. I'm certainly not an authority here :-) --- I now also tested your patch. I noticed the following issues: > http-response del-header -m beg This should probably cause a configuration parsing failure, but is silently accepted. I am not sure what is does, delete headers called `-m`, ignoring the `beg`? Best regards Tim Düsterhus
