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

[feature] add 'Model::add_calendar' under 'mutable-model' feature #761

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

woshilapin
Copy link
Contributor

@woshilapin woshilapin commented Mar 31, 2021

Introduce a new function to mutate the model.

Model::add_calendar(
  &mut self,
  calendar: Calendar,
  // Other parameters ?
)

@woshilapin
Copy link
Contributor Author

This PR depends on hove-io/relational_types#11. However, there is still a bunch of discussions about that other PR and no consensus. Therefore, I'm not going to rely on this new OneToMany::add_link() function, but I'm going to recreate entirely the relation every time with OneToMany::new(). This might be less performant, but it will be always correct and performance improvement can still be addressed later if needed.

@woshilapin woshilapin force-pushed the model_add_calendar branch 3 times, most recently from a9a7c55 to f02244b Compare April 2, 2021 17:34
@woshilapin woshilapin marked this pull request as ready for review April 2, 2021 17:48
@pbougue pbougue requested review from pbougue and ArnaudOggy April 6, 2021 08:52
ArnaudOggy
ArnaudOggy previously approved these changes Apr 6, 2021
pbougue
pbougue previously approved these changes Apr 7, 2021
@woshilapin woshilapin dismissed stale reviews from pbougue and ArnaudOggy via a3b1a9a April 7, 2021 12:30
@woshilapin woshilapin force-pushed the model_add_calendar branch from a3b1a9a to cfb2caf Compare April 7, 2021 12:31
@woshilapin woshilapin merged commit a6d6460 into master Apr 7, 2021
@woshilapin woshilapin deleted the model_add_calendar branch April 7, 2021 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants