Skip to content

Convert encodable() methods to From implementations #3124

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

Merged
merged 4 commits into from
Dec 28, 2020

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Dec 26, 2020

Our JSON views and the database models are currently tightly coupled through the encodable() methods on almost all models.

Instead of having the database models depend on the JSON views it would be better to reverse this relationship, so that the database models can live on their own, and only the JSON views are depending on the database models. This would eventually allow us to extract the database models and schema into a dedicated crate, which would then allow us to cut down the dependency tree of some second-level binaries like monitor and background-worker

This PR starts the process of moving away from encodable() by implementing the From trait for two of our JSON views. This allows us to call .into() on the models to automatically convert it to the JSON view form.

r? @pietroalbini

@pietroalbini
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 28, 2020

📌 Commit 7baa13e has been approved by pietroalbini

@bors
Copy link
Contributor

bors commented Dec 28, 2020

⌛ Testing commit 7baa13e with merge 2d82b5a...

@bors
Copy link
Contributor

bors commented Dec 28, 2020

☀️ Test successful - checks-actions
Approved by: pietroalbini
Pushing 2d82b5a to master...

@bors bors merged commit 2d82b5a into rust-lang:master Dec 28, 2020
@Turbo87 Turbo87 deleted the encodable branch December 28, 2020 09:55
bors added a commit that referenced this pull request Dec 28, 2020
Replace more `encodable()` methods with `From` implementations

This PR continues #3124 by replacing more of our `encodable()` methods with `From` trait implementations to ultimately decouple our database models from the JSON views.

r? `@pietroalbini`
bors added a commit that referenced this pull request Jan 7, 2021
models::Dependency: Replace `encodable()` method

This PR continues #3124 by implementing `from_dep()` and `from_reverse_dep()` methods for the `EncodableDependency` struct and removing the `encodable()` methods on `Dependency` and `ReverseDependency`. We can't use the `From` trait in these cases because the `crate_name` has to be explicitly provided. What we could do instead is implement `From<(Dependency, String)>`, but that doesn't feel right to me.

r? `@pietroalbini`
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.

4 participants