On Tue, Jul 19, 2022 at 05:21:51PM +0200, Omar Polo wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
> > There is another subtle difference. When we call got_object_commit_create()
> > in worktree.c's commit_worktree(), we call time(NULL) twice. This could mean
> > that timestamp info between author and committer could differ, provided
> > committer is not NULL (otherwise, the result of the second time(NULL) will
> > be ignored). This should probably be fixed to pass a single time_t variable
> > into got_object_commit_create() twice instead?
> 
> that's a good point.  yes, commit_worktree should be fixed not to call
> time twice for sure.  I'll look into it soon, I was thinking of looking
> at preserving the committer info during histedit.

Yes. And indeed this code might need to be modified when making
such changes to histedit.

> > Thinking about this in terms of UI:
> > We assume that people would only use commit -A if they were committing
> > someone else's work, right? If so, should we make it an error if the
> > -A flag tries to set the same value redundantly? Such that this would
> > always error out: got commit -A "$GOT_AUTHOR"
> > Would that be helpful, or would it be annoying?
> 
> this is another good question.  i'm not 100% convinced, but i'm inclined
> toward raising an error.

Works for me.

> P.S.: there's a way to build a "throw-away" got_error?  it would allow
> me to avoid defining a new error type for this corner case.

Error codes are cheap. Just add another :)