From: http://anastigmatix.net/ Date: Fri, 24 Oct 2014 00:40:35 +0000 (-0400) Subject: Feeling out how to present patch for review X-Git-Tag: 3.20150107~103 X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/commitdiff_plain/1fc5cf6fb92013907d37f5506381eae53ca1ad44 Feeling out how to present patch for review --- diff --git a/doc/todo/Let_plugins_influence_what_environment_variables_a_wrapper_will_preserve.mdwn b/doc/todo/Let_plugins_influence_what_environment_variables_a_wrapper_will_preserve.mdwn index 1626367e3..48901a2c4 100644 --- a/doc/todo/Let_plugins_influence_what_environment_variables_a_wrapper_will_preserve.mdwn +++ b/doc/todo/Let_plugins_influence_what_environment_variables_a_wrapper_will_preserve.mdwn @@ -1,12 +1,14 @@ -I created this patch in advance of writing the [[plugins/contrib/signinview]] plugin. This patch does nothing `signinview`-specific, but simply refactors wrapper generation a bit so that plugins have some influence over what environment variables a wrapper will preserve. +[[!template id=gitbranch branch=jcflack/config-envsave +author="[[Chapman Flack|jcflack]]" +browse=https://github.com/joeyh/ikiwiki/pull/14]] -For example, Wrapper.pm previously hardcoded not only (some of) the RFC 3875 variables needed for a CGI wrapper (and hardcoded its own test for _whether it was generating_ a CGI wrapper), but also the Apache ErrorDocument-specific variables needed by the [[plugins/404]] plugin. Given that `signinview`, as a `403` handler, would have similar requirements to `404`, and it seemed possible other wrappers for other purposes could rely on other environment variables too, it seemed to make sense to move the preserved-variable list out of Wrapper.pm hardcoding, and closer to the plugins or other code relying on the variables. +I created this [[!taglink patch]] in advance of writing the [[plugins/contrib/signinview]] plugin. This patch does nothing `signinview`-specific, but simply refactors wrapper generation a bit so that plugins have some influence over what environment variables a wrapper will preserve. -More comments and the code changes for review are [here](https://github.com/joeyh/ikiwiki/pull/14). +For example, Wrapper.pm previously hardcoded not only (some of) the RFC 3875 variables needed for a CGI wrapper (and hardcoded its own test for _whether it was generating_ a CGI wrapper), but also the Apache ErrorDocument-specific variables needed by the [[plugins/404]] plugin. Given that `signinview`, as a `403` handler, would have similar requirements to `404`, and it seemed possible other wrappers for other purposes could rely on other environment variables too, it seemed to make sense to move the preserved-variable list out of Wrapper.pm hardcoding, and closer to the plugins or other code relying on the variables. ---- -I was asked by [[Joey]] to create a page here for purposes of review. I'm still trying to figure out the preferred workflow for this project ... I'm assuming the link above is ok for looking over the comments and code changes, since they're already pushed to my git fork and (to me, anyway) reviewing changes in a decent repository browser is so much nicer than a `diff -u` pasted into a page between `
` tags.
+I was asked by [[Joey]] to create a page here for purposes of review. I'm still trying to figure out the preferred workflow for this project ... I'm assuming the github link is ok for looking over the comments and code changes, since they're already pushed to my git fork and (to me, anyway) reviewing changes in a decent repository browser is so much nicer than a `diff -u` pasted into a page between `
` tags.
 
-That seemed to be ok for reviewing [[bugs/CGI wrapper doesn't store PERL5LIB environment variable]], so I hope it's ok for this one too.
+That seemed to be ok for reviewing [[bugs/CGI wrapper doesn't store PERL5LIB environment variable]], so I hope it's ok for this one too. If another way would be preferable, please let me know.
 
 -- [[jcflack]]