Jonathan Nieder <jrnieder@xxxxxxxxx> writes: >>> [...] >> >> What does HT stands for? I will change the indentation to 8 spaces. > > HT means "horizontal tab", like might be shown with "man ascii". > > Git uses tabs for indentation. This file is documentation instead of > source so clang-format doesn't know about it, but I might as well > mention anyway: if you run "make style", then clang-format will give > some suggestions around formatting. The configuration for that is not > yet perfect so you can take its suggestions with a grain of salt, but > they should get you in the right direction. > Got it, thanks! >[...] >>>> --- a/builtin/commit.c >>>> +++ b/builtin/commit.c >>>> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) >>>> status_format = STATUS_FORMAT_NONE; /* Ignore status.short */ >>>> s.colopts = 0; >>>> >>>> + git_config(git_default_config, NULL); >>> >>> Declaration after statement is not tolerated in this codebase. >> >> If I use the DEVELOPER=1 flag in config.mak and call make again, would the compiler >> catches this as an error? > > Yes, DEVELOPER_CFLAGS includes -Wdeclaration-after-statement. > Got the error from int hide_timezone = 0; but not git_config(git_default_config, NULL);. >>>> + int hide_timezone = 0; >>> >>> Unnecessary initialization. >>> >>>> + if (!git_config_get_bool("user.hideTimezone", &hide_timezone) && hide_timezone) >> >> Is it unnecessary because I am checking the return value from git_config_get_bool so >> that the uninitialized value won't be used? > > By leaving it uninitialized, you can help avoid the reader wondering > whether there is some code path where the default value is used. > I see. >[...] >>> Instead, make sure it is set to some timestamp in some >>> timezone that is not UTC, and the timezone of the resulting commit >>> author date is in that timezone. But that must have already been >>> done in basic tests on "git commit" that we honor the environment >>> variable, no? Which means there is no need to add yet another extra >>> baseline test here. >> >> I am not sure if this test has already been done in commit basic tests. >> Will remove this test. > > Let's see: *checks with "git grep -e TZ -- t"*. > > Looks like t0006 tests various aspects of TZ handling pretty well and > t1100 includes of test using TZ with commit-tree (good). > > Thanks and hope that helps, > Jonathan Thanks! That helps a lot.