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

Update C# codegen to consistent filtering rules #1277

Merged
merged 3 commits into from
May 28, 2024

Conversation

RReverser
Copy link
Contributor

Description of Changes

API and ABI breaking changes

Limits equatable types and changes return type of FilterBy functions as per proposal.

Expected complexity level and risk

2

How complicated do you think these changes are? Grade on a scale from 1 to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex change.

This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.

If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning ways.

Testing

Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!

  • Built & ran the client-side chat example in C# SDK.
  • Ran codegen tests and updated the snapshot.

Copy link
Contributor

@kazimuth kazimuth left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Small issue

@RReverser RReverser requested a review from jdetter May 28, 2024 12:57
RReverser added 3 commits May 28, 2024 15:00
- Limit types as per proposal.
- Add `Query` client-side SDK helper for API parity with server-side modules (on client-side it's a simple wrapper around Iter + Where).
- Change return type of `FilterBy` to always be iterable, with new `FindBy` function for unique fields.
- Simplify the way primary keys are handled - must go with clockworklabs/com.clockworklabs.spacetimedbsdk#93 for the client SDK counterpart.
@RReverser RReverser force-pushed the ingvar/consistent-filtering-csharp-client branch from 8f3a6b8 to 8ac9272 Compare May 28, 2024 14:07
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Import issue fixed, thanks 👍

@RReverser RReverser added this pull request to the merge queue May 28, 2024
RReverser added a commit to clockworklabs/com.clockworklabs.spacetimedbsdk that referenced this pull request May 28, 2024
## Description of Changes

As a result of the consistent filtering rules proposal
(clockworklabs/SpacetimeDB#1256), all equatable types are now natively
equatable in C# as well.

This allows us to compare objects directly, without holding and
comparing AlgebraicValue in the entries map as well, which has a bit of
a domino effect and allows to optimise, simplify or even remove some
parts of the SDK.

## API

 - [x] This is an API breaking change to the SDK

New filtering rules limit types on which filtering can be done, as well
as change the return type of `FilterBy` functions to always be iterable.

## Requires SpacetimeDB PRs

clockworklabs/SpacetimeDB#1277
Merged via the queue into master with commit 307dfee May 28, 2024
6 checks passed
@RReverser RReverser deleted the ingvar/consistent-filtering-csharp-client branch May 28, 2024 22:54
RReverser added a commit that referenced this pull request May 28, 2024
Fixes regression accidentally introduced in #1277: if FindBy returns null, FilterBy will return an iterable with a single null item instead of an empty iterable.
@RReverser RReverser mentioned this pull request May 28, 2024
2 tasks
github-merge-queue bot pushed a commit that referenced this pull request May 31, 2024
* Fix FilterBy regression in C#

Fixes regression accidentally introduced in #1277: if FindBy returns null, FilterBy will return an iterable with a single null item instead of an empty iterable.

* Fix snapshots
@bfops bfops added the api-break A PR that makes an API breaking change label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break A PR that makes an API breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants