You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 2001/09/10 22:08:28 UTC
merge macros (was: Re: mod_ssl broken)
On Mon, Sep 10, 2001 at 09:23:59AM -0500, William A. Rowe, Jr. wrote:
> From: "Ryan Bloom" <rb...@covalent.net>
> Sent: Monday, September 10, 2001 8:39 AM
>
> > > I don't mind putting a patch together that does this (and to use
> > > it in all core modules). I would appreciate suggestions for the
> > > final names though (naming isn't my strong side).
> > >
> > > AP_CFG_MERGE
> > > AP_CFG_MERGE_ARRAY
> >
> > Don't do that. That is just Ralf's coding style. It is not proof that we need this
> > in every single core module.
>
> HUH? The point is that even subversion uses such helpers for clarity. It would
> help all authors to offer these, consistently.
>
> We've (collectively) proven that config merge errors are simple to author, and
> difficult to debug.
And the SVN merge macros came from mod_dav. The only reason mod_dav_fs
doesn't have a similar macro is that it only has a single item to merge, so
I just spelled it out.
So you could say that I'm hitting 3 for 3 with using that INHERIT_VALUE
macro, and would appreciate a core version of it.
AP_CFG_MERGE_* located in http_config.h would make some sense to me.
In the mod_ssl version, there was the "unset" param; I would suggest the
simplest for assumes NULL. A second one would take an "unset" param.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: merge macros
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Sep 10, 2001 at 03:18:35PM -0700, Ryan Bloom wrote:
> On Monday 10 September 2001 15:11, Sander Striker wrote:
> > Ryan, was that a -1, or just a 'don't invest time in it, I don't
> > think it's worth it'?
>
> The latter. I would never veto something like this, I just don't believe that
> it is required.
Translated as, "I wouldn't spend time on it, but your time is yours -- feel
free spend your time on it if you want."
IMO, things that clarify or make the codebase more consistent are always a
good thing. In some cases, not enough of this happens and the codebase
begins to show its creakiness. Adding comments, increasing consistency,
refactoring, etc.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: merge macros (was: Re: mod_ssl broken)
Posted by Ryan Bloom <rb...@covalent.net>.
On Monday 10 September 2001 15:11, Sander Striker wrote:
> Ryan, was that a -1, or just a 'don't invest time in it, I don't
> think it's worth it'?
The latter. I would never veto something like this, I just don't believe that
it is required.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
RE: merge macros (was: Re: mod_ssl broken)
Posted by Sander Striker <st...@apache.org>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: 10 September 2001 22:08
> On Mon, Sep 10, 2001 at 09:23:59AM -0500, William A. Rowe, Jr. wrote:
> > From: "Ryan Bloom" <rb...@covalent.net>
> > Sent: Monday, September 10, 2001 8:39 AM
> >
> > > > I don't mind putting a patch together that does this (and to use
> > > > it in all core modules). I would appreciate suggestions for the
> > > > final names though (naming isn't my strong side).
> > > >
> > > > AP_CFG_MERGE
> > > > AP_CFG_MERGE_ARRAY
> > >
> > > Don't do that. That is just Ralf's coding style. It is not
> proof that we need this
> > > in every single core module.
> >
> > HUH? The point is that even subversion uses such helpers for
> clarity. It would
> > help all authors to offer these, consistently.
> >
> > We've (collectively) proven that config merge errors are simple
> to author, and
> > difficult to debug.
>
> And the SVN merge macros came from mod_dav. The only reason mod_dav_fs
> doesn't have a similar macro is that it only has a single item to
> merge, so
> I just spelled it out.
>
> So you could say that I'm hitting 3 for 3 with using that INHERIT_VALUE
> macro, and would appreciate a core version of it.
>
> AP_CFG_MERGE_* located in http_config.h would make some sense to me.
>
> In the mod_ssl version, there was the "unset" param; I would suggest the
> simplest for assumes NULL. A second one would take an "unset" param.
>
> Cheers,
> -g
Ryan, was that a -1, or just a 'don't invest time in it, I don't
think it's worth it'?
Consistency wise I would find it usefull to put a patch together.
But if I know beforehand that it is not going in, I'd rather defer
and use my energy elsewhere.
Sander