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

Revise Boxes 📦 #249

Merged
merged 1 commit into from
Feb 9, 2022
Merged

Revise Boxes 📦 #249

merged 1 commit into from
Feb 9, 2022

Conversation

p4checo
Copy link
Member

@p4checo p4checo commented Jan 17, 2022

Checklist

Motivation and Context

We have had two Box variants for a very long time: Box and VarBox, containing an immutable and mutable value respectively.

However, from our experience the immutable variant is not used often if at all, which means its immutability guarantee is not something that people want or need.

Furthermore, following the latest language developments a Box is a very good candidate for a property wrapper, and more so now that they can be defined as local properties.

Description

  • Deprecate VarBox in favor of Box, making it wrap a mutable value.

  • Make Box a @propertyWrapper, and @dynamicMemberLookup. The latter is useful if we use Box<T> as a regular type (i.e. not a PW).

@p4checo
Copy link
Member Author

p4checo commented Jan 17, 2022

It seems CI is breaking because it's using macOS-11 image, which currently still compiles with Xcode 12.5. That will apparently change on January 24th, so we can wait a couple of days to avoid messing with CI to override the active Xcode version.

An official macOS-12 image should pop up eventually as well 🙏🏼

We have had two `Box` variants for a very long time: `Box` and `VarBox`,
containing an immutable and mutable value respectively.

However, from our experience the immutable variant is not used often if
at all, which means its immutability guarantee is not something that
people want or need.

Furthermore, following the latest language developments a `Box` is a
very good candidate for a property wrapper, and more so now that they
can be defined as local properties.

## Changes

- Deprecate `VarBox` in favor of `Box`, making it wrap a mutable value.

- Make `Box` a `@propertyWrapper`, and `@dynamicMemberLookup`. The
latter is useful if we use `Box<T>` as a regular type (i.e. not a PW).
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@5dc9b0f). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #249   +/-   ##
=========================================
  Coverage          ?   92.02%           
=========================================
  Files             ?       99           
  Lines             ?     3284           
  Branches          ?        0           
=========================================
  Hits              ?     3022           
  Misses            ?      262           
  Partials          ?        0           
Impacted Files Coverage Δ
...urces/Network/Network+URLSessionNetworkStack.swift 93.58% <100.00%> (ø)
Sources/Utils/Box.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dc9b0f...92978a9. Read the comment docs.

@p4checo p4checo merged commit fc34695 into master Feb 9, 2022
@p4checo p4checo deleted the revise-boxes branch February 9, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants