git-number cannot commit-tree without user config

git-number cannot commit-tree without user config

user.name and/or user.email may be not configured, so specify them
explictly for commit-tree.

R=iannucci@chromium.org
BUG=354276
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=261256

[nodir, 2014-04-02 18:34:38]

+agable

[agable, 2014-04-02 23:29:46]

https://codereview.chromium.org/222103006/diff/1/git_number.py
File git_number.py (right):

https://codereview.chromium.org/222103006/diff/1/git_number.py#newcode137
git_number.py:137: # Git user.name and/or user.email may not be configured, so
specifying them
Nonono. These environment variables, if present, override any configuration in
the user's .gitconfig. At the very least we should only set these environment
variables if we absolutely have to. Even better, we should simply make sure all
of our systems have .gitconfig set up properly.

[nodir, 2014-04-02 23:41:54]

1) I don't think we should make sure all of our systems ahve .gitconfig. The
problem I am having is that gitpoller workdir doesn't have a .gitconfig.
GitHelper does not configure user.name and user.email when clones a repo.

2) Yes, the override, so what? These values are not used, and even if they were,
the commit is not made by the person who configured the system.

[agable, 2014-04-03 00:53:13]

On 2014/04/02 23:41:54, nodir wrote:
> 1) I don't think we should make sure all of our systems ahve .gitconfig. The
> problem I am having is that gitpoller workdir doesn't have a .gitconfig.
> GitHelper does not configure user.name and user.email when clones a repo.

Yes, and my point was that there's no reason for masterX.golo to not have a
.gitconfig.

> 2) Yes, the override, so what? These values are not used, and even if they
were,
> the commit is not made by the person who configured the system.

Chatted with Robbie, he convinced me that user.name and user.email should have
to be configured for git number because it's an read-only inspection utility
(like git show / log) and the fact that it uses commits for caching is
irrelevant. So okay, let's override everywhere.

That said, we shouldn't override like this. The commit command should be
git -c user.name=<value> user.email=<value> commit <args>

[agable, 2014-04-03 00:54:55]

On 2014/04/03 00:53:13, agable wrote:
> On 2014/04/02 23:41:54, nodir wrote:
> > 1) I don't think we should make sure all of our systems ahve .gitconfig. The
> > problem I am having is that gitpoller workdir doesn't have a .gitconfig.
> > GitHelper does not configure user.name and user.email when clones a repo.
> 
> Yes, and my point was that there's no reason for masterX.golo to not have a
> .gitconfig.
> 
> > 2) Yes, the override, so what? These values are not used, and even if they
> were,
> > the commit is not made by the person who configured the system.
> 
> Chatted with Robbie, he convinced me that user.name and user.email should have

s/should/shouldn't/

> to be configured for git number because it's an read-only inspection utility
> (like git show / log) and the fact that it uses commits for caching is
> irrelevant. So okay, let's override everywhere.
> 
> That said, we shouldn't override like this. The commit command should be
> git -c user.name=<value> user.email=<value> commit <args>

[nodir, 2014-04-03 01:30:43]

On 2014/04/03 00:54:55, agable wrote:
> On 2014/04/03 00:53:13, agable wrote:
> > On 2014/04/02 23:41:54, nodir wrote:
> > > 1) I don't think we should make sure all of our systems ahve .gitconfig.
The
> > > problem I am having is that gitpoller workdir doesn't have a .gitconfig.
> > > GitHelper does not configure user.name and user.email when clones a repo.
> > 
> > Yes, and my point was that there's no reason for masterX.golo to not have a
> > .gitconfig.
> > 
> > > 2) Yes, the override, so what? These values are not used, and even if they
> > were,
> > > the commit is not made by the person who configured the system.
> > 
> > Chatted with Robbie, he convinced me that user.name and user.email should
have
> 
> s/should/shouldn't/
> 
> > to be configured for git number because it's an read-only inspection utility
> > (like git show / log) and the fact that it uses commits for caching is
> > irrelevant. So okay, let's override everywhere.
> > 
> > That said, we shouldn't override like this. The commit command should be
> > git -c user.name=<value> user.email=<value> commit <args>

you probably meant 

git -c user.name=<value> -c user.email=<value> commit <args>

Done.

[agable, 2014-04-03 01:33:07]

On 2014/04/03 01:30:43, nodir wrote:
> On 2014/04/03 00:54:55, agable wrote:
> > On 2014/04/03 00:53:13, agable wrote:
> > > On 2014/04/02 23:41:54, nodir wrote:
> > > > 1) I don't think we should make sure all of our systems ahve .gitconfig.
> The
> > > > problem I am having is that gitpoller workdir doesn't have a .gitconfig.
> > > > GitHelper does not configure user.name and user.email when clones a
repo.
> > > 
> > > Yes, and my point was that there's no reason for masterX.golo to not have
a
> > > .gitconfig.
> > > 
> > > > 2) Yes, the override, so what? These values are not used, and even if
they
> > > were,
> > > > the commit is not made by the person who configured the system.
> > > 
> > > Chatted with Robbie, he convinced me that user.name and user.email should
> have
> > 
> > s/should/shouldn't/
> > 
> > > to be configured for git number because it's an read-only inspection
utility
> > > (like git show / log) and the fact that it uses commits for caching is
> > > irrelevant. So okay, let's override everywhere.
> > > 
> > > That said, we shouldn't override like this. The commit command should be
> > > git -c user.name=<value> user.email=<value> commit <args>
> 
> you probably meant 
> 
> git -c user.name=<value> -c user.email=<value> commit <args>
> 
> Done.

Yep, good catch. LGTM.

[nodir, 2014-04-03 01:41:45]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2014-04-03 01:41:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nodir@chromium.org/222103006/40001

[commit-bot: I haz the power, 2014-04-03 01:43:33]

Change committed as 261256