Remove constraints from default_main where possible.
#96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If we derive `Clone` manually for `ServerState`, we can avoid the unnecessary constraint on connectors implementing `Clone`.
These are so basic for passing configuration and state around across async boundaries that we should just require them in the trait itself.
SamirTalwar
requested review from
plcplc,
paf31,
daniel-chambers,
soupi and
BenoitRanque
plcplc
reviewed
Feb 13, 2024
plcplc
reviewed
Feb 13, 2024
| C::RawConfiguration: Serialize + DeserializeOwned + JsonSchema + Sync + Send, | ||
| C::Configuration: Serialize + DeserializeOwned + Sync + Send + Clone, | ||
| C::State: Sync + Send + Clone, | ||
| C::RawConfiguration: Serialize + DeserializeOwned + JsonSchema, |
There was a problem hiding this comment.
Taking the argument of this PR a bit further, is there any sense to any impl Connector for C that doesn't satisfy the constraints of default_main?
A connector that doesn't might as well not use ndc-sdk at all.
There was a problem hiding this comment.
Well, you can make your own main, in theory. I was wondering this but didn't want to push it too far.
There was a problem hiding this comment.
Yeah, maybe. I'm just wondering what if anything anyone who makes their own main function actually gets from using the sdk.
At any rate this is still a fine improvement.
plcplc
approved these changes
Feb 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have made a few changes here.
Sync + Sendto the connector type constraints, because they're so basic that I can't imagine a connector working without them.CloneonServerState.