Skip to content

Replace a do-catch block with Result(catching:)#19259

Merged
mokagio merged 1 commit intocore-data-adopt-the-new-writer-api-part-2from
mokagio/core-data-result-refinement
Aug 31, 2022
Merged

Replace a do-catch block with Result(catching:)#19259
mokagio merged 1 commit intocore-data-adopt-the-new-writer-api-part-2from
mokagio/core-data-result-refinement

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Aug 31, 2022

This Result initializer takes a throwing closure as input and returns a Result wrapping its result.

See https://developer.apple.com/documentation/swift/result/init(catching:)

Regression Notes

  1. Potential unintended areas of impact – The behavior of the method I edited is covered by unit tests. If I made a mistake, the test will tell us.
  2. What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
  3. What automated tests I added (or what prevented me from doing so) – N.A.

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes. N.A.
  • I have considered adding accessibility improvements for my changes. N.A.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

This `Result` initializer takes a throwing closure as input and returns
a `Result` wrapping its result.

See https://developer.apple.com/documentation/swift/result/init(catching:)
@mokagio mokagio added this to the 20.7 milestone Aug 31, 2022
@mokagio mokagio self-assigned this Aug 31, 2022
@mokagio mokagio requested a review from crazytonyli August 31, 2022 04:11
} catch {
completion?(.failure(error))
}
completion?(result)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crazytonyli I find this more readable (and indulgent in using Result API 🤓 ).

YMMV. Given the functionality doesn't change, I won't mind if you'd rather keep your version 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! I didn't know this function exists.

@wpmobilebot
Copy link
Contributor

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19259-320b58f on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19259-320b58f on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@mokagio mokagio merged commit d3c4c96 into core-data-adopt-the-new-writer-api-part-2 Aug 31, 2022
@mokagio mokagio deleted the mokagio/core-data-result-refinement branch August 31, 2022 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants