Random Refactoring

[dahlbyk]

Miscellaneous changes that came to mind while poking around...

1. Two files where we deliberately ignore C# naming conventions. If you'd rather not add R#-specific error suppression, that's cool too.
2. Setting up an equality helper's particularly noisy; params cleans it up muchly.
3. I started just making defaultValue optional, which feels right. Removing the two-part overload was necessary (and makes sense, I think) because of a conflict with the three-part overload when retrieving a string.
4. Consuming the _foreach APIs feels clumsy and repetitive, so returning a collection from Proxy seems like a good move. The implementation's a bit weird, though (especially the conversion between delegate types) - any suggestions are welcome.

[dahlbyk]

New addition: nullable config values so it's possible to differentiate between an unset value and a set value that matches false/0. Trivial to support, perhaps even useful.

Related: passing null to Set<string>() results in unsetting a value, but it's not clear from our documentation that that's the expected behavior. If it is, should we also support Set<int?>(key, null), etc?

[nulltoken]

New addition: nullable config values so it's possible to differentiate between an unset value and a set value that matches false/0. Trivial to support, perhaps even useful.

I like this new feature very much. Maybe should we get rid of (ie. Mark [Obsolete]) the overload which provides a default value?

Related: passing null to Set() results in unsetting a value, but it's not clear from our documentation that that's the expected behavior. If it is, should we also support Set<int?>(key, null), etc?

Huh. That makes me feel a bit uneasy. Looks like a side-effect. I'd rather have no feature overlap between Set and Unset. Could we guard against this?

[haacked]

Why would we mark anything Obsolete? LibGit2Sharp isn't 1.0 yet, right? Just get rid of it. :) After all, there's already been some breaking changes recently and we've just adapted. :)

[nulltoken]

Why would we mark anything Obsolete?

We try to minimize the breaking changes and provide the user with hints.

[haacked]

@nulltoken Will these be removed before 1.0 is stamped? It'd be odd to have a 1.0 with Obsolete methods.

[haacked]

Speaking of which, if you change an error message contents, that could be a potential breaking change for us. So do let us know. :)

[dahlbyk]

Speaking of which, if you change an error message contents, that could be a potential breaking change for us. So do let us know. :)

Those spots are probably an indication that we need a specific exception type.

@nulltoken Will these be removed before 1.0 is stamped? It'd be odd to have a 1.0 with Obsolete methods.

I'd say take them out between 1.0-RC (or whatever) and 1.0.

[haacked]

Indeed. We've wanted Exception types or Error codes for a while now. :)

[nulltoken]

@haacked

Will these be removed before 1.0 is stamped? It'd be odd to have a 1.0 with Obsolete methods.

1.0 is not there yet :) Those [Obsolete] will be dropped as soon as v0.10 is released.

@dahlbyk has already worked on a PR to take care of this. cf. #181.

If some breaking changes are made once v0.10 is released, they will be marked [Obsolete] and dropped after v0.11 is released...

If you change an error message contents, that could be a potential breaking change for us. So do let us know. :)

Hum. I'm not sure I can commit to this. I'll try to pay attention though and cc you each an every time I see such changes.

Indeed. We've wanted Exception types or Error codes for a while now. :)

If you feel that some Exceptions are missing, please update #189 accordingly.

[haacked]

@nulltoken Yeah, I wouldn't expect you to commit to that. But a heads up if you remember is nice. In the meanwhile, I'll compile an exhaustive list of error cases we need to detect and update #189.

:)

[nulltoken]

I'll compile an exhaustive list of error cases we need to detect and update #189.

❤️

[nulltoken]

@dahlbyk I've cherry picked ccaa34c, 163bb04 and 61e9f3b into vNext.

It looks like 67fa534 is embedded in #243.

Regarding c975958, it's being discussed in #243 as well.

Thanks for being that awesome!

[dahlbyk]

Cool, closing.