👍 to drop those overloads. Although they're convenient for a demo session, they're not very safe to be used and have bitten us in the past.

/cc @dahlbyk @ethomson @Jameson @Therzok

👍

Transmuted this issue into a PR with a proposal. Thoughts?

As a consumer of the API, completely removing the Commit(message) overload is severely inconvenient. Passing a signature along with every commit is kind of clunky.

You're still free to assume that BuildSignature() is going to find a signature, or create an extension method in your code which assumes your environment is always going to be set up just right. But that'd be an assumption you make about your environment, not something the library encourages.

All I'm saying is that it's not very elegant. I understand why it's needed, but it also doesn't make for a very nice API.

Perhaps a property added to the Repository object?

using (var repo = New Repository)
{
    repo.CurrentSignature = new Signature("rubberduck", "rubberduck@Foobar.com")

   //... do some things

   repo.Commit("message")
}

I'm not a huge fan of that idea, because then Commit would have to check the property and throw an invalid operation exception if it was null. It makes assumptions about the state of the object that may not be true. Just thinking out loud about trying to find a more elegant solution.

@ckuhn203 brings up a good point. Even if we would consider it a best practice to explicitly specify an author, a git commit with name/email unset does exactly what we do now.

Beware of trying to implement the git tool's UI. It can change without much notice, and this bit specifically has done so a few times in the past as it's meant for humans, not code.

git commit with name/email unset does exactly what we do now.

If there is absolutely nothing configured, git will continue and create a commit with whatever it can find out of the current machine name and user account, which is going to be the wrong thing more often than not, but it errs on the side of making progress which is presumably better for the first steps (it does also tell you how to set the configuration). This does however mean that many people end up wanting to do a filter-branch of the first commits because their name isn't right on there.

What git does here is policy at the application layer, and what happens depends on it being a command-line application. If I were using a GUI without the user/email configuration set up, I would expect it to prompt me for the information. If I'm using some automation tool which performs a commit server-side, I would expect it to ask who to impersonate or use its name, as that information is much more useful than what system account it happens to run under. If it's a client-side automation tool, it's probably OK for it to use whatever is in the config, but I would expect it to be configurable.

All of these decisions are down to what the tool does and in what context is runs. We cannot pretend to know what is right for the consumers of the library. I also don't feel particularly compelled to make it convenient not to care about what information goes into a rather visible part of immutable history data. It also goes against making the right thing easer than the wrong thing. You can still make it completely dependent on the configuration by passing the config-generated signature, but it's at least as easy to put in new Signature("my tool", "my-tool@example.com"). Even better if this also goes with an explanation in the Commit() param that this is the signature for the current user or an identifier for the tool.

You've convinced me. We'll just need to update the wiki if/when this is implemented. I can see it causing a lot of confusion if it's not part of the Quick Start.

Ah yes, we have one of those on this repo. Yes, the quick start would be a great place to say something like "the username from the config, or ask the user if you're committing on behalf of a user; if you're writing a tool, it's a good idea to identify it here so you know where the change came from".

👍