Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bindgen] std:optional is preferred over util::Optional #7284

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

kneth
Copy link
Contributor

@kneth kneth commented Jan 23, 2024

What, How & Why?

std::optional is preferred over util::Optional in core, and bindgen should adapt to the convention.

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)
  • C-API, if public C++ API changed
  • bindgen/spec.yml, if public C++ API changed

@kneth kneth requested a review from jedelbo January 23, 2024 17:05
@fealebenpae
Copy link
Member

I believe we want to deprecate util::Optional and encourage new code to use std::optional instead. I think we should rather update the binding generator to support std::optional than keep using util::Optional in the spec, even though we use it less and less everywhere else.

Copy link

coveralls-official bot commented Jan 23, 2024

Pull Request Test Coverage Report for Build kenneth.geisshirt_2

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 60 unchanged lines in 11 files lost coverage.
  • Overall coverage increased (+0.001%) to 91.841%

Files with Coverage Reduction New Missed Lines %
src/realm/index_string.hpp 1 82.86%
src/realm/array_backlink.cpp 2 95.9%
src/realm/array_blobs_big.cpp 2 98.72%
src/realm/sync/client.cpp 3 91.1%
src/realm/sync/network/network.cpp 3 89.66%
test/test_util_network.cpp 3 97.2%
src/realm/unicode.cpp 4 90.15%
test/fuzz_group.cpp 8 53.72%
src/realm/sync/transform.cpp 9 62.56%
src/realm/sync/noinst/client_impl_base.cpp 10 85.16%
Totals Coverage Status
Change from base Build 1998: 0.001%
Covered Lines: 235274
Relevant Lines: 256174

💛 - Coveralls

@jedelbo
Copy link
Contributor

jedelbo commented Jan 26, 2024

@kneth Can't you just replace all util::Optional in spec.yml with std::optional? In core we have
using util::Optional = std::optional
so there is actually no util::Optional anymore.

@kneth kneth force-pushed the kneth/bindgen/use-util-optional branch from dcab590 to fc07fa8 Compare January 30, 2024 13:04
@cla-bot cla-bot bot added the cla: yes label Jan 30, 2024
@kneth kneth changed the title [bindgen] util::Optional is preferred over std::optional [bindgen] std:optional is preferred over util::Optional Jan 30, 2024
@kneth
Copy link
Contributor Author

kneth commented Jan 30, 2024

@jedelbo @fealebenpae I have migrated bindgen to std::optional

@kneth kneth merged commit f7edd47 into master Jan 31, 2024
34 of 38 checks passed
@kneth kneth deleted the kneth/bindgen/use-util-optional branch January 31, 2024 11:07
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants