Conversation
|
|
||
| let itemsInValidRenderGroups = NSSet( | ||
| array: renderGroups.lazy | ||
| let itemsInValidRenderGroups = Set( |
There was a problem hiding this comment.
Right now this hashes by class identity instead of by value identity, right?
How about we change this to Set<ObjectIdentifier> instead by doing .map { ObjectIdentifier($0) } and then changing the check below to: !itemsInValidRenderGroups.contains(ObjectIdentifier(item.item)).
This would avoid changing the runtime semantics, and would avoid the need to add a custom Hashable conformance.
There was a problem hiding this comment.
Yep, I had a similar thought after playing around with it more.
There was a problem hiding this comment.
PTAL. I could add a ShapeLayer test, but it looks like this is covered by existing tests.
There was a problem hiding this comment.
np about tests, I trust this will be covered by the snapshot tests
|
I'll figure out what broke out GH actions CI, probably an actions runner change |
|
It was just a flake, great |
2 participants
Setinstead ofNSSetforitemsInValidRenderGroups.