Re: [RFC PATCH 1/1] hideTimezone: add a user.hideTimezone config
- From
- Shengfa Lin <shengfa@google.com>
- Date
- Oct 2, 2020, 21:17 UTC
- Message-ID
- <20201002211719.68606-1-shengfa@google.com>
- In-Reply-To
- <20201001024452.GA2930867@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 9 quoted lines
> Like Junio mentioned, this affects "git commit" but not other commands > that record the current date with the local timezone. > > The fundamental tool to exercise that machinery is > > $ git var GIT_AUTHOR_IDENT > Jonathan Nieder <jrnieder@gmail.com> 1601517809 -0700 > > so I suppose I'd be interested in seeing that exercised in tests.
Next patch should include a test for git var.
Show 9 quoted lines
> [...] > > The unfortunate thing about these APIs is that there's no way to pass > in a timezone from a string instead of from the environment. This > means that passing through the environment as above is the only > reasonable way to do it, but that would have the unfortunate result > of changing the output of commands like "git log --date=local" that > are about writing dates to the terminal instead of storing them. >
The TZ should apply for write but not read.
Show 16 quoted lines
> [...] > Looking over callers, who would this affect? There are three callers: > > fast-import.c::parse_ident: > Used to handle ident string "now". That seems in keeping with > the intent here, and fast-import does respect some other > configuration though only affecting storage. Seems fine.sensible. > > ident.c::ident_default_date: > Used to produce author and committer timestamps and timestamps for > reflog entries. That's the goal; good. > > send-pack.c::generate_push_cert: > Used for the timestamp sent to the server in a signed push > certificate. Also good. >
Thanks for the analysis here. I was wondering whether the TZ should affect these callers.
Show 10 quoted lines
> So I think this does the right thing, plus it retains the > user-friendly feature of being able to *display* timestamps in their > local timezone. > > Now let's talk through the downsides: > > It's complex. The performance isn't likely to be bad when > user.timezone is not set, which is nice, but it still is messier than > I'd like to see. >
+1.