clockworklabs / clockworklabs/SpacetimeDB

Reimplement: [C#] NFC: split out IStructuralWrite interface

Open
#4,787 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

release-any
Dominant language
Rust
Stars
25.2k
Forks
1.1k
Avg merge
2d 7h
Merged PRs (30d)
46

Description

This issue tracks reimplementation of the work from stale PR #1717, which is being closed because it is too out of date to merge directly.

Original PR: https://github.com/clockworklabs/SpacetimeDB/pull/1717
Original author: @RReverser
Original branch: `ingvar/structural-write`
Base branch: `master`

## Original PR summary

Structural read implementation can be only directly implemented on types with default constructors, but writing should not have such limitations.

In this PR I'm splitting out writing into its own interface, so that it can be implemented even on types like tagged enums, which simplifies their own BSATN.Write implementation and allows to use them with helpers like ToBytes.

I'm also making the generated tagged enum record abstract - which it, arguably, should've been from the beginning - but it should be a non-functional change because its constructor has always been private anyway.

Description of Changes

Please describe your change, mention any related tickets, and so on here.

API and ABI breaking changes

If this is an API or ABI breaking change, please apply the
corresponding GitHub label.

Expected complexity level and risk

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!

  • Write a test you've completed here.

  • Write a test you want a reviewer to do here, so they can check it off when they're satisfied.

    Follow-up

    • Reimplement this change in a fresh PR against current master.
    • Carry forward any still-relevant context from the original PR discussion and review.
    • Link the new implementation PR back to the original stale PR for historical context.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing stale PR #1717 and its discussion, then compare the proposed changes with current master. Reimplement the relevant C# structural writing interface split and generated tagged-enum adjustment in a fresh PR, carrying forward applicable tests and context from the original work.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.