Re: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM
On Tue, Apr 16, 2013 at 12:36 AM, Junio C Hamano wrote: > Drew Northup writes: > >>> + Note that the GITWEB_CONFIG_SYSTEM system-wide configuration file is >>> + only used for instances that lack per-instance configuration file. >>> + You can use GITWEB_CONFIG_COMMON common system-wide configuration >>> + file (normally /etc/gitweb-common.conf) to keep common default >>> + settings that apply to all instances. Settings from per-instance or >>>system-wide configuration file override those from common system-wide >>>configuration file. >> >> That's the point of explaining SPECIFICALLY why the then current >> behavior wasn't being replaced, and this other mechanism (which would >> otherwise have no obvious reason for existing) was being introduced. > > In order to just pick and use the more appropriate one (or a useful > combination of the two), a clean description of what each of them do > without historical cruft is more readable and useful, isn't it? I am not demanding the retention of cruft, and the rewording is definitely more pleasant to read. > I > would expect that most of them who are newly configuring a system > would pick COMMON one and override per instance as needed, without > touching the SYSTEM one (fallback default) after reading the above, > and that is what we want to happen. > > Do you think sysadmins need a history lesson to understand why there > are two different possibilities? We don't need a full history lesson. What we do need to know is that "Hey, they don't do that the way everybody else does because it would break things." That's enough to get the point across, and as Jakub noted gitweb.conf.txt is the correct place for it. > For example, bash reads some but not all possible configuration > files. I would expect .bashrc to be read even for login shells after > reading .bash_login; alas, that is not what happens. The manual > does not apologize that the authors now know better and understand > that it is a stupid behaviour. The order the rc files are read is > just described matter-of-factly, and it gives sufficient information > without unnecessary backstory. > > I think the new text conveys the necessary information to the > intended audience with more clarity without the history lesson or > the record of your past frustration. Am I mistaken? The back-story isn't needed; the "Hey this is different" part is. I think Jakub's suggestion of covering that (succinctly) in gitweb.conf.txt is the correct solution. -- -Drew Northup -- "As opposed to vegetable or mineral error?" -John Pescatore, SANS NewsBites Vol. 12 Num. 59 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM
On Tue, Apr 16, 2013 at 3:11 AM, Jakub Narębski wrote: > Junio C Hamano wrote: > >> In order to just pick and use the more appropriate one (or a useful >> combination of the two), a clean description of what each of them do >> without historical cruft is more readable and useful, isn't it? I >> would expect that most of them who are newly configuring a system >> would pick COMMON one and override per instance as needed, without >> touching the SYSTEM one (fallback default) after reading the above, >> and that is what we want to happen. >> >> Do you think sysadmins need a history lesson to understand why there >> are two different possibilities? > [...] >> I think the new text conveys the necessary information to the >> intended audience with more clarity without the history lesson or >> the record of your past frustration. Am I mistaken? > > Note also that this is about *gitweb/INSTALL*, which is meant to be > *short* and succint description on how to install gitweb, and not > about the reference documentation: gitweb(1) or gitweb.conf(5). > > Description of historical behavior (and backward compatibility) > has place (if any) in manpages, not gitweb/INSTALL. > -- > Jakub Narębski Let us then agree that it should be mentioned somewhere in gitweb.conf.txt then (as it currently is not). -- -Drew Northup -- "As opposed to vegetable or mineral error?" -John Pescatore, SANS NewsBites Vol. 12 Num. 59 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM
Junio C Hamano wrote: > In order to just pick and use the more appropriate one (or a useful > combination of the two), a clean description of what each of them do > without historical cruft is more readable and useful, isn't it? I > would expect that most of them who are newly configuring a system > would pick COMMON one and override per instance as needed, without > touching the SYSTEM one (fallback default) after reading the above, > and that is what we want to happen. > > Do you think sysadmins need a history lesson to understand why there > are two different possibilities? [...] > I think the new text conveys the necessary information to the > intended audience with more clarity without the history lesson or > the record of your past frustration. Am I mistaken? Note also that this is about *gitweb/INSTALL*, which is meant to be *short* and succint description on how to install gitweb, and not about the reference documentation: gitweb(1) or gitweb.conf(5). Description of historical behavior (and backward compatibility) has place (if any) in manpages, not gitweb/INSTALL. -- Jakub Narębski -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM
Drew Northup writes: >> + Note that the GITWEB_CONFIG_SYSTEM system-wide configuration file is >> + only used for instances that lack per-instance configuration file. >> + You can use GITWEB_CONFIG_COMMON common system-wide configuration >> + file (normally /etc/gitweb-common.conf) to keep common default >> + settings that apply to all instances. Settings from per-instance or >>system-wide configuration file override those from common system-wide >>configuration file. > > That's the point of explaining SPECIFICALLY why the then current > behavior wasn't being replaced, and this other mechanism (which would > otherwise have no obvious reason for existing) was being introduced. In order to just pick and use the more appropriate one (or a useful combination of the two), a clean description of what each of them do without historical cruft is more readable and useful, isn't it? I would expect that most of them who are newly configuring a system would pick COMMON one and override per instance as needed, without touching the SYSTEM one (fallback default) after reading the above, and that is what we want to happen. Do you think sysadmins need a history lesson to understand why there are two different possibilities? For example, bash reads some but not all possible configuration files. I would expect .bashrc to be read even for login shells after reading .bash_login; alas, that is not what happens. The manual does not apologize that the authors now know better and understand that it is a stupid behaviour. The order the rc files are read is just described matter-of-factly, and it gives sufficient information without unnecessary backstory. I think the new text conveys the necessary information to the intended audience with more clarity without the history lesson or the record of your past frustration. Am I mistaken? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM
On Fri, Apr 12, 2013 at 6:20 PM, Jakub Narębski wrote: > -- >8 -- > Subject: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM > > The flow of the text describing GITWEB_CONFIG_SYSTEM and > GITWEB_CONFIG_COMMON in gitweb/INSTALL is awkward. "This is > bad. Oh the other hand, better is broken. Therefore ..." forces > readers to make multiple guesses while reading: "ok, bad, so you plan > to change it and warn us about upcoming change? oh, not that, > changing it is bad, so we have to live with it? oh, not that, there > is another one that is common and that is what we can use". > > Better rewrite said paragraph to avoid such a mental roller-coaster in > the first place. > > Signed-off-by: Junio Hamano > Signed-off-by: Jakub Narebski > --- > gitweb/INSTALL | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/gitweb/INSTALL b/gitweb/INSTALL > index 6d45406..7ad1050 100644 > --- a/gitweb/INSTALL > +++ b/gitweb/INSTALL > @@ -243,14 +243,12 @@ for gitweb (in gitweb/README), and gitweb.conf(5) > manpage. >GITWEB_CONFIG_SYSTEM build configuration variable, and override it >through the GITWEB_CONFIG_SYSTEM environment variable. > > - Note that if per-instance configuration file exists, then system-wide > - configuration is _not used at all_. This is quite untypical and suprising > - behavior. On the other hand changing current behavior would break > backwards > - compatibility and can lead to unexpected changes in gitweb behavior. > - Therefore gitweb also looks for common system-wide configuration file, > - normally /etc/gitweb-common.conf (set during build time using build time > - configuration variable GITWEB_CONFIG_COMMON, set it at runtime using > - environment variable with the same name). Settings from per-instance or > + > + Note that the GITWEB_CONFIG_SYSTEM system-wide configuration file is > + only used for instances that lack per-instance configuration file. > + You can use GITWEB_CONFIG_COMMON common system-wide configuration > + file (normally /etc/gitweb-common.conf) to keep common default > + settings that apply to all instances. Settings from per-instance or >system-wide configuration file override those from common system-wide >configuration file. The point of wording it such that it was explicitly noted that the supposed "system-wide" settings were really just "system-wide defaults" (which would be ignored wholesale if any one setting was overridden locally) was due to the fact that most of the time server administrators don't deal with software that idiosyncratic. Prior to the addition of GITWEB_CONFIG_COMMON there was no method for setting up a sane default template that wouldn't be discarded wholesale upon any one configuration parameter being overridden. That's the point of explaining SPECIFICALLY why the then current behavior wasn't being replaced, and this other mechanism (which would otherwise have no obvious reason for existing) was being introduced. So, unfortunately, if we remove the "mental roller coaster" part of the explanation entirely we can expect to end up in exactly the situation that I complained about to begin with. (Or we could just do this the way everybody else does, with partial overrides being the common case, starting at 2.x and no longer have the mental roller coaster problem at all.) As I'm the one that complained loudly enough to get this change to be made in the first place I'd appreciate being kept in the loop in this series. -- -Drew Northup -- "As opposed to vegetable or mineral error?" -John Pescatore, SANS NewsBites Vol. 12 Num. 59 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM
On Fri, 12 April 2013, Junio C Hamano wrote: > Jonathan Nieder writes: > >>Note that if per-instance configuration file exists, then system-wide >> - configuration is _not used at all_. This is quite untypical and suprising >> + configuration is _not used at all_. This is quite untypical and >> surprising >>behavior. On the other hand changing current behavior would break >> backwards >>compatibility and can lead to unexpected changes in gitweb behavior. >>Therefore gitweb also looks for common system-wide configuration file, > > Hmm, "atypical", isn't it? > > The flow of the text is awkward. "This is bad. Oh the other hand, > better is broken. Therefore ..." forces readers to make multiple > guesses while reading: "ok, bad, so you plan to change it and warn > us about upcoming change? oh, not that, changing it is bad, so we > have to live with it? oh, not that, there is another one that is > common and that is what we can use". > > It may be a good idea to rewrite this paragraph to avoid such a > mental roller-coaster in the first place. > > The GITWEB_CONFIG_SYSTEM system-wide configuration file is only > used for instances that lack per-instance configuration file. > You can use GITWEB_CONFIG_COMMON file to keep common default > settings that apply to all instances. > > or something. > > Not asking for a re-roll, but it may be a potential follow-up candidate. Perhaps something like this? Note that this change avoids repetition of build / environmental configuration variable (I think the paragraph above, not touched in this patch, also might need rewrite). -- >8 -- Subject: [PATCH] gitweb/INSTALL: Simplify description of GITWEB_CONFIG_SYSTEM The flow of the text describing GITWEB_CONFIG_SYSTEM and GITWEB_CONFIG_COMMON in gitweb/INSTALL is awkward. "This is bad. Oh the other hand, better is broken. Therefore ..." forces readers to make multiple guesses while reading: "ok, bad, so you plan to change it and warn us about upcoming change? oh, not that, changing it is bad, so we have to live with it? oh, not that, there is another one that is common and that is what we can use". Better rewrite said paragraph to avoid such a mental roller-coaster in the first place. Signed-off-by: Junio Hamano Signed-off-by: Jakub Narebski --- gitweb/INSTALL | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/gitweb/INSTALL b/gitweb/INSTALL index 6d45406..7ad1050 100644 --- a/gitweb/INSTALL +++ b/gitweb/INSTALL @@ -243,14 +243,12 @@ for gitweb (in gitweb/README), and gitweb.conf(5) manpage. GITWEB_CONFIG_SYSTEM build configuration variable, and override it through the GITWEB_CONFIG_SYSTEM environment variable. - Note that if per-instance configuration file exists, then system-wide - configuration is _not used at all_. This is quite untypical and suprising - behavior. On the other hand changing current behavior would break backwards - compatibility and can lead to unexpected changes in gitweb behavior. - Therefore gitweb also looks for common system-wide configuration file, - normally /etc/gitweb-common.conf (set during build time using build time - configuration variable GITWEB_CONFIG_COMMON, set it at runtime using - environment variable with the same name). Settings from per-instance or + + Note that the GITWEB_CONFIG_SYSTEM system-wide configuration file is + only used for instances that lack per-instance configuration file. + You can use GITWEB_CONFIG_COMMON common system-wide configuration + file (normally /etc/gitweb-common.conf) to keep common default + settings that apply to all instances. Settings from per-instance or system-wide configuration file override those from common system-wide configuration file. -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
