Better documentation for edge cases of `commit` command

[yuhr]

isomorphic-git/isomorphic-git#643 (comment)

I'm kind of confused by lots of thoughts 🤔

For instance, what if:

commit({ ...,
  author: {
    name: 'someone',
    email: '...',
    date: new Date('August 19, 2012 23:15:30')
  },
  committer: {
    name: 'me'
    email: '...'
  }
})

In this case, I guess the user has intended to set author's date to several years ago but apparently still wants to commit just now as himself, because the docs page says nothing about such a case though seeing author.date it says just "Default is the current date". But current implementation won't do that. This way the user is likely to get a misdated commit object accidentally. I don't know what is your design decision actually @wmhilton @mojavelinux, anyway I just followed to this behaviour in #643 with 5bc1d66.

Here's another instance:

commit({ ...,
  author: {
    name: 'someone',
    email: '...'
  }
})

We expect the auther to be also the committer in this case, but what if the user didn't read the docs well and was to commit as himself using default values in .git/config, not author? This case is pretty hard to prevent from having unexpected commits. Although I don't think such a tiny edge case is worth supporting, acually.

Also what if:

commit({ ...,
  author: {
    name: 'someone',
    email: '...'
  },
  committer: {
    name: 'me'
    email: '...',
    date: new Date('August 19, 2012 23:15:30')
  }
})

I can't imagine the case author's date is later than committer's, however I'm not sure whether we should default it to committer's or now. For API consistency, we may want to choose the latter.

So... even though we don't make such breaking changes, we may want to write some explicit notes with highlighted style in the docs about them, and of course it's also possible to throw some errors for such ambiguous parameter patterns.

[yuhr]

Oh, tricky users might do this kind of thing 🤣

commit({ ...,
  author: {
    name: 'someone',
    email: '...'
  },
  committer: {}
})

Well. The silver bullet could be to make parameters required, if it's not breaking things too much lol.

[billiegoose]

Yeah... and the scenario I was trying to mentally calculate was what happens if:

commit({ ...,
  author: {
    name: 'someone',
    email: '...',
    date: new Date('Thu Dec 13 2018 22:47:25 GMT-0500')
  },
  committer: {
    name: 'me'
    email: '...',
    timestamp: 1544759187
  }
})

They would end up with the committer using the provided timestamp... but the time ZONE from author instead of the current time zone.

[billiegoose]

I'm not sure what to do about these situations. Maybe just improve the docs? Right now all it says for committer is "If not specified, the author details are used." If we expanded it to list each field and indicated the default value, then it would be more clear.

[mojavelinux]

Maybe just improve the docs?

That would be my philosophy here. There's a point when you have to specify what's ambiguous and trust the user of the API to pass in sensible values.

[yuhr]

Let's go with that! 😄