Re: [PATCH 2/2] gitweb: introduce localtime feature

[Jakub Narebski <jnareb@gmail.com>]

On Thu, Mar 17, 2011 at 21:12, Kevin Cernekee wrote:
> On Thu, Mar 17, 2011 at 4:01 AM, Jakub Narebski <jnareb@gmail.com> wrote:

> > This does not describe why would one want such way of displaying
> > timestamps, and which views would be affected.
> >
> > BTW. should it be timezone of web server (machine where gitweb is
> > run), or local time of author / committer / tagger as described in the
> > timezone part of git timestamp?
> 
> The case I am currently trying to improve is the one in which all
> developers are at a single site.
> 
> In the open source world it is common to have developers scattered all
> over the globe, so some of them will inevitably have to perform
> timezone conversions.
> 
> But Git is becoming a popular tool in the private sector and it is
> common to have most/all contributors based in a single office.  In the
> latter case, it is helpful to display the local timezone instead of
> GMT.  This also helps make the data more readable by program managers
> and other non-developers who have an interest in tracking the project.

Such explanation should in my opinion be present in the proposed commit
message.  Good commit message should explain not only what the change is
intent to do (to catch when implementation and intent differs), but also
whys behind the change (to find whether commit is worth having).

The above nicely explains why and when such feature would be useful,
 
> > Why project specific override is not supported?  I think it might make
> > sense to enable this feature on project-by-project basis; some
> > projects might be dispersed geographically, some might not.
> 
> Mostly ease of testing.  I did not need it for any of my projects.

You mean that for your instance of gitweb all projects were single
office (not dispersed geographically), so for you enabling it site-wide
was enough, isn't it?
 
> It turned out to be a simple change, and it is in v2.

Thanks.

> The cases I tested were: 
> 
> default 0
> default 1
> override 1, project unset
> override 1, project 0
> override 1, project 1

Well, I think the non-overridden / overridden feature we have tested quite
well.  BTW did you add test for 'localtime' feature to t9500 test?  Perhaps
it is not strictly necessary, though...

> > Is it still an RFC 2822 conformant date?  If it is not, then above
> > change is invalid, and we have to implement this feature in different
> > way.
> 
> I believe it is still valid.
> 
> Original date: Thu, 17 Mar 2011 02:11:05 +0000
> New date: Wed, 16 Mar 2011 19:11:05 -0700
> Sample date from RFC 2822 Appendix A: Fri, 21 Nov 1997 09:55:06 -0600

Thanks.

> > Hmmm... I wonder if it wouldn't be better to print both times (perhaps
> > reversed) in this case...
> 
> I have submitted a third patch which does this.

Note that we print localtime (time in author / committer / tagger timezone)
to be able to mark given time as "atnight", so one can easily see commits
and tags which needs more careful review because they were made 4 AM or
something.

If you reverse the direction you still should make sure that "atnight"
styling applies to localtime.

-- 
Jakub Narebski
Poland