Return Result values rather than converting errors to None#1241
Conversation
DanielEScherzer
force-pushed
the
return-results
branch
from
b043aae to
b82963e
Compare
|
Now that #1239 has been merged, we should probably return |
DanielEScherzer
force-pushed
the
return-results
branch
from
b82963e to
ba8cb90
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
This is nice, though also non-utf8 path is rare. I guess we might want to bundle this with libgit2 v2 change, as that would be a larger release having more API changes. What do you think?
There was a problem hiding this comment.
I think that there is a benefit to doing this now, so that callers can know what the error was instead of just getting back an opaque None, but ultimately it isn't my call. I just want to say that without a timeline for libgit2 v2 I wouldn't want to delay this indefinitely
There was a problem hiding this comment.
Yeah reasonable. Fine making some breaking changes now than later. Though still want to point how non-utf8 is not that common.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
This may need a rebase onto main branch |
* `Commit::message()` * `Commit::message_encoding()` * `Commit::message_raw()` * `Commit::raw_header()` * `Commit::summary()` * `Commit::body()`
* `ConfigEntry::name()` * `ConfigEntry::value()`
* `PushUpdate::src_refname()` * `PushUpdate::dst_refname()`
* `Reference::name()` * `Reference::shorthand()` * `Reference::symbolic_target()`
* `Refspec::dst()` * `Refspec::src()` * `Refspec::str()`
* `Remote::name()` * `Remote::url()` * `Remote::pushurl()`
* `Signature::name()` * `Signature::email()`
* `Submodule::branch()` * `Submodule::url()` * `Submodule::name()`
* `Tag::message()` * `Tag::name()`
DanielEScherzer
force-pushed
the
return-results
branch
from
ba8cb90 to
8fa1eb0
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
3 participants
Fixes #857