`Gollum::Git::Index#commit` does not set `:committer`

[jhominal]

As the title indicates, Gollum::Git::Index#commit does not set the :committer option.

As a comparison, Grit sets committer in Grit::Index#commit to the same value as author.

I have only hit that issue because, as the :committer value is not set, Rugged attempted to use the configured value, which was empty in my situation, and thus threw an exception:

Rugged::ConfigError - Config value 'user.name' was not found

What is the target behavior?

• Committer is set to the same value as Author;
  • That would follow the same behavior as grit;
• Committer is set to a default value;
  • That would also be fine - after all, the actual entity committing to the repository is not the wiki user, but the gollum software;
• rugged_adapter does not work unless user.name is configured;
  • That would be less than ideal, but is the current behavior;

If the behavior changes, how should it be specced?

• Should we check that the expected value of committer on the returned Gollum::Git::Commit object?
  • However, committer information is not part of the Adapter API, as it is unnecessary for Gollum;
• Should we check that a call to commit will not raise an exception?
  • However, the conditions for the exception to occur require that user.name not be set, on any level - however, a developer running the specs is extremely likely to have user.name set in his user file, making the spec useless;
• Do we spec this behavior only for rugged_adapter, not in adapter_specs?

Yes, I know I am writing far too much for a one-line change...

[bartkamphorst]

I know I am writing far too much for a one-line change...

No, this is great; I'm happy to have the rugged adapter scrutinized carefully. My initial reaction is to make the rugged adapter follow the grit adapter as closely as possible, unless we think that behavior makes no sense either.

[jhominal]

Thank you.

I will add that RJGit also sets both author and committer to the same value. So, I guess we should just agree to implement the same behavior in rugged_adapter.

The only question remaining for me then, is whether, and how, to spec that change.

[bartkamphorst]

The only question remaining for me then, is whether, and how, to spec that change.

Best is probably to add a test to gollum-lib's test suite. That way, we have one test for all adapters.

[gerwitz]

@jhominal I owe you a drink for fixing this.