On Tue, 2011-06-07 at 08:44 -0500, Jonathan Nieder wrote: > Hi, > > Drew Northup wrote: > > On Mon, 2011-06-06 at 17:12 -0500, Jonathan Nieder wrote: > > >> Micronit: a single line like > >> > >> SYNOPSIS > >> -------- > >> $GITWEBDIR/gitweb_config.perl, /etc/gitweb.conf > >> > >> might fit better with the pattern established by gitattributes(5) and > >> its kin. > > > > I thought that's how I had originally put it. > > I take that as a "yes, it would work better". Agreed, it would. > > To save others the trouble of digging previous rounds up for > comparison: > > - the original: http://thread.gmane.org/gmane.comp.version-control.git/173422 > - v2: http://thread.gmane.org/gmane.comp.version-control.git/174700/focus=174701 > - v3: http://thread.gmane.org/gmane.comp.version-control.git/175140/focus=175145 > > The original had one line but it didn't mention > $GITWEBDIR/gitweb_config.perl. So we are slowly converging. Thanks for looking that up (and the refs above). > > [...] > >>> +The configuration file is used to override the default settings that > >>> +were built into gitweb at the time 'gitweb.cgi' script was generated. > >>> + > >>> +While one could just alter the configuration settings in the gitweb > >>> +CGI itself, those changes would be lost upon upgrade. > >> > >> What if I upgrade using RCS "merge"? :) So maybe: > >> > >> s/one could just alter/one can alter/ > >> s/would be lost/could be lost/ > >> > >> to clarify that (1) editing the CGI script is allowed and fine but (2) > >> it might be a pain to keep those changes. > > > > The point of this was originally if you were a mere mortal then upgrades > > of the gitweb CGI would flush all of the settings you just spent a week > > getting right down the drain. That's the point of the stronger language. > > It's meant to be a solid "If you do it that way you can expect it to > > hurt" message. > > Sorry, I suppose I was unclear. All I meant is that as a naÃve > reader, I found myself losing trust with "those changes would be lost > upon upgrade". Oddly enough, by contrast something like: > > While it would be possible to change the gitweb CGI directly > instead, DON'T DO THAT! Your changes would be lost on upgrade! > > would have worked fine for me, since there are enough cues to know I > am reading hyperbole rather than an explanation of mechanism. > > Anyway, it was just how I read it, and it's likely my proposed change > would have made it worse in some other way. Your complaint is valid from the perspective you just outlined. The point I was trying to drive home was that you can expect the upgrade process to replace the executable because that's what upgrades do--they replace executables. Obviously stating it that plainly to somebody unfamiliar with how the whole install/upgrade process works at the filesystem level isn't going to be very helpful. Perhaps a combination of the "one can alter" and "are likely to be overwritten [upon upgrade]" would do the trick? > > >>> Configuration > >>> +settings might also be placed into a file in the same directory as the > >>> +CGI script with the default name 'gitweb_config.perl' -- allowing > >>> +one to have multiple gitweb instances with different configurations by > >>> +the use of symlinks. > >> > >> Might also in addition to what? > > > > This comment is nonsensical, please clarify it. > > I found the sentence starting with "Configuration settings" hard to > understand because the "also" didn't have an obvious antecedent. > Therefore I proposed some alternative phrasing: > > >> Continuing the thought from before: > >> > >> So gitweb allows settings to be placed in a separate file named > >> 'gitweb_config.perl' in the same directory as the CGI script. > >> This also allows one to set up multiple gitweb instances with > >> different configurations by using symlinks to a common gitweb.cgi > >> script. > > > > How is this any different from what I wrote? > > I'm starting to sense annoyance. Please feel free to ignore my > comments if you think they're unhelpful; they're meant as a gift, not > demands. > > Anyway, the difference is in phrasing. Saying "So" is meant to make > the relationship to the previous sentence clearer: we are explaining > the purpose of the configuration file, by contrasting it with editing > the CGI script directly. The next sentence is a separate thought, so > I thought it might be clearer to put it in a separate sentence. Yes, annoyance would be the right word. I was trying very hard to find how what you were proposing was different and not finding it (other than what appeared to me to be dealing with the reader in a disrespectful manner). Now that I know where you were aiming I can think about an appropriate change. (I'm not there yet, but it may very well come to me.) In any case, these are things that I'd need to see not in patch context, but as part of the larger file to make much sense of. I seem to recall that something is missing here that was prior to the words "Configuration settings" in my original, which I'll have to check on. If that is the case it would explain the lack antecedent clarity you noted. If not then the text could be made more clear unto itself. > >>> +The location of system-wide gitweb configuration file is defined at compile > >>> +time using the configuration value `GITWEB_CONFIG_SYSTEM` and defaults to > >>> +'/etc/gitweb.conf'. The name of the per-instance configuration file is > >>> +defined in gitweb by `GITWEB_CONFIG`, and defaults to 'gitweb_config.perl' > >>> +(relative path means located in the same directory gitweb is installed). > >> > >> Maybe: > >> > >> In addition to the per-instance configuration file, there can > >> be a system-wide configuration file to act as a fallback when > >> the per-instance configuration file does not exist. > >> > >> The system-wide configuration file is named /etc/gitweb.conf > >> by default. Filenames for the system-wide and per-instance > >> configuration variables can be overridden at compile time and > >> run time; see the FILES section for details. > > > > This is the manpage for the system wide configuration file. If you'd > > like to scrap this effort in favor of something else please speak up. > > Huh? If this is what you are proposing then we should be working on a "gitweb_config.perl" manpage and not a "gitweb.conf" manpage. I know a fair number of people around here put priority on the former and would just as soon ignore the latter. That's what your proposed change says to me (while I also understand that your own position is likely far more nuanced than that). > > >>> +*NOTE:* If per-instance configuration file ('gitweb_config.perl') exists, > >>> +then system-wide configuration file ('/etc/gitweb.conf') is _not used at > >>> +all_!!! > >> > >> Over the top. :) I think the best way to document this is to contrast > >> it with /etc/gitweb-common.conf once the latter exists. > > > > If we were to change gitweb to handle configuration files like the rest > > of git (and in fact like most things we deal with daily, where settings > > are overridden one by one) then this section becomes moot. Until or > > unless that becomes the case it is important to loudly make note of it. > > Does using three exclamation marks and italics make it clearer? That could probably be cut down to one, I'll grant you that. I was trying to avoid use of the <blink> tag. > > Previously there was no manpage documenting this at all, so I think > explaining it is already a big and good step. If this needs to be > so prominent that people just skimming through will notice it, then > I'd suggest putting it in the DESCRIPTION section higher up. But I'm > not the one writing this; others are free to disagree. I thought seriously about doing that but decided against it as it took this important bit of information out of the appropriate context. If I don't present that there's more than one way to configure gitweb first this text by itself doesn't make a whole lot of sense (and therefore will likely be ignored). > > >>> +The syntax of the configuration files is that of Perl, as these files are > [...] > >>> +page for more information. > >> > >> Duplicates DESCRIPTION. > > > > Perhaps it does, but not everybody is a Perl coder. It was that way in > > the text I started from and I saw little point in changing it. > > No, the paragraph is actually repeated from the DESCRIPTION section. Well then, I suppose that is a problem! I don't recall doing that, but it should be corrected. (As noted below and prior, I haven't been able to apply the interim patches to my @home working copy, so if it came from there I currently don't know it.) > > [...] > >>> +The default configuration with no configuration file at all may work > >>> +perfectly well for some installations. Still, a configuration file is > >>> +useful for customizing or tweaking the behavior of gitweb in many ways, and > >>> +some optional features will not be present unless explicitly enabled using > >>> +the configurable `%features` variable (see also "Configuring gitweb > >>> +features" section below). > >> > >> I suppose this is based on the text > >> > >> The most notable thing that is not configurable at compile time are the > >> optional features, stored in the '%features' variable. > >> > >> I'd suggest removing the paragraph. <snip> > > > > Apparently you think that clearing these misconceptions up is a useless > > exercise, or at least it sounds a lot like it. > > What? I wrote exactly what I was thinking, which was that I thought this > paragraph was based on the text I quoted from gitweb/INSTALL and that > contrasting the run-time and compile-time configuration as that > paragraph did didn't seem very important for the installed manpage > (since the reader might not have been involved in the installation at > all). > > If the point was actually to say "Contrary to popular belief, you can > set %features in the configuration file instead of hard-coding it in > the CGI script itself", why not say that directly? Ok, we can do that (or something very much like it, whatever ends up fitting best). Remember that this was sourced prior to our on-list discussion about what came from gitweb/INSTALL being primarily fodder for a different document. I haven't had the chance to modify it since, and I have the impression that Jakub has been busy working on other parts of this project (of which he's done quite a bit). Removing the paragraph wholesale doesn't fix the problem so far as I see it, but the presentation of the information can certainly be better tuned. > > [...] > > I cannot keep up with this pace of patching right now. > > No problem. For what it's worth, I skipped a couple of rounds of > reading, too; I suppose they came quickly. Does that mean you don't > want to be cc-ed on later incarnations of the patch, or would you like > to stay notified? I'd like to stay cc'd, I just haven't been able to apply the previous patch versions to my @home work tree before the next has come out, limiting my chances to be able to effectively review them. I apologize for being a bit snappy, but sometimes I really do have a low tolerance for comments that just seem (to me) to be "this here is really useless" complaints and not suggestions of an upgrade. -- -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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html