Move model into a separate rspotify-model crate

[udoprog]

Description

This extracts the models used into a separate crate, so they can be used separately from the client implementation.

Motivation and Context

I have a separate client which is not suitable for general use that does not need the client bits, but could benefit greatly from the models.

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

This also requires that the next release of rspotify releases the rspotify-model crate. The separated crate is being re-exported with the same API as before. A few models have been modified from being pub(in crate) to just be pub.

How Has This Been Tested?

Just built for now. I don't have the necessary plumbing to run the integration tests, so we'll see if they pass through Github Actions.

[marioortizmanero]

I like the idea. Other libraries are much more modular than rspotify, see bevy.

But shouldn't we use workspaces rather than a crate in src and another in model? Maybe it'll be more organized that way

[udoprog]

But shouldn't we use workspaces rather than a crate in src and another in model? Maybe it'll be more organized that way

Yeah. Just making the change as small as possible right now. Setting up a workspace would mean moving rspotify proper into its own directory which I'm open to doing once this has gotten a green light.

[marioortizmanero]

Sounds good to me, we just need to know @ramsayleung's opinion. We could move some parts of the code into crates, like the HTTP client as rspotify-http, but not sure that's worth our time. Let's keep it simple for now, yeah. By the way, may I know why you need the model but not the client? Not sure what your use case is.

The only part I can see getting worse is the documentation. We would have to add a link to rspotify-model's crate on the main docs or something. It might be harder to upload new versions and such, but I guess that's nothing a good GitHub Actions workflow can't fix.

Also, if you want to fix the CI you should merge with master.

[udoprog]

The only part I can see getting worse is the documentation. We would have to add a link to rspotify-model's crate on the main docs or something. It might be harder to upload new versions and such, but I guess that's nothing a good GitHub Actions workflow can't fix.

With the rspotify crate we can at least mark the re-export with #[doc(inline)]. Then rustdoc will pretend that whatever types are being re-exported are part of the same crate, so the experience for the current crate should be the same.

Also, if you want to fix the CI you should merge with master.

Will do, thanks!

[marioortizmanero]

With the rspotify crate we can mark the re-export with #[doc(inline)]. Then rustdoc will pretend that whatever types are being re-exported are part of the same crate, so the experience should be the same.

Oh that is great, thanks. We have some models that are supposedly private (those with pub (in crate) previously). To keep them that way we could use #[doc(hidden)].

Also, you answered just after I edited my comment, I think you missed this:

By the way, may I know why you need the model but not the client? Not sure what your use case is.

[udoprog]

By the way, may I know why you need the model but not the client? Not sure what your use case is.

Sure. I already have a client that is integrated into the project https://github.com/udoprog/OxidizeBot/blob/main/bot/src/api/spotify/mod.rs. It uses an 17 month old copy of the models provided in rspotify.

[marioortizmanero]

Sure. I already have a client that is integrated into the project https://github.com/udoprog/OxidizeBot/blob/main/bot/src/api/spotify/mod.rs. It uses an 17 month old copy of the models provided in rspotify.

Oh ok that makes sense. Why not just use rspotify entirely in this case though? You'd save some maintainance work that way. Anyhow, I like the idea, thanks for contributing.

[ramsayleung]

It's a good idea to seperate rspotify-model :) I agree with Mario's point:

But shouldn't we use workspaces rather than a crate in src and another in model?

src and model don't stand on the same level of abstraction, IMHO, it will be better to use workspace.

We could move some parts of the code into crates, like the HTTP client as rspotify-http, but not sure that's worth our time.

Or we could refactor the HTTP client into rspotify-http crate and move authenticate process into rspotify-auth? I am not sure about that either.

Since the architecture of rspotify package would look like this:

We could seperate rspotify into crates like this:

• rspotify-model
• rspotify-oauth
• rspotify-http
• rspotify-client

or

• rspotify-model
• rspotify-http
• rspotify-client

How about this idea?

[udoprog]

src and model don't stand on the same level of abstraction, IMHO, it will be better to use workspace.

As indicated earlier I don't have anything against doing the move as long as thing change is approved. But since I won't be using either the client, auth, or http components I'm not best suited for moving the other crates. I also don't know if there is an expressed need for breaking things up further, but this is of course up for you to decide.

I can try and introduce a workspace and do rspotify-model though if you want. But as indicated earlier, it will be a lot of changes and they'll make any open PRs outdated. I'd also stress that in principle, there's nothing preventing keeping things outside of a workspace structure in the interim.

[marioortizmanero]

Don't worry, we were just discussing rspotify-oauth and rspotify-client to work on it later. I have to rewrite part of the OAuth as #173 indicates, so I can take care of that. We can discuss that and figure it out later, but I like how Ramsay's diagram looks.

I don't see that many problems by introducing workspaces. It should be as easy as a rename, adding cargo files, and perhaps changing the CI a bit, and we can help you with that. These things shouldn't conflict with the rest of the PRs.

We should merge #166 and #189 before this, but I still don't think we'll have that many conflicts. As to #176 and #178, I will probably take them into account with the OAuth rewrite and copy parts of them, but I don't think they should be merged now.

[marioortizmanero]

Any progress on this, @udoprog?

[udoprog]

Any progress on this, @udoprog?

No. Anyone else interested in doing the split can feel free to!

[marioortizmanero]

Sure, I'll give it a try this weekend if I have time.

[ramsayleung]

Are you guys working on split this crate into workspace? If not, I could take time to do that :)

[marioortizmanero]

Hi! Yes, I tried it two weeks ago, but failed to, in this branch https://github.com/ramsayleung/rspotify/tree/separate-crates. I split up the library into rspotify-http, rspotify-auth and rspotify-client as well. But that ended up being impossible because they are tied together and won't work in separate crates at all until #173 is done.

So we should only have rspotify-client with these three crates combined, and after #173 we can separate them again.

I was going to try to do that but I didn't end up having enough time. You can give it a try if you want, as the branch is still there and you can make a new one based on separate-crates. I was planning on doing that myself this weekend but not sure if I'll be able to either.

Another thing to take into account is that the GitHub Actions workflows should be modified, because the features are now for the specific crates, and cargo test --workspace --feature=client-ureq doesn't work anymore. But it should be as easy as making a workflow for each crate.

I was implementing the whole thing inspired on how https://github.com/bevyengine/bevy does it, which I really like in terms of how it's structured. You can take a look as well for your attempt if you like.

Sorry about that!

[ramsayleung]

Since you are working on separating this library into individual crates, we could wait util #173 is merged :)

[marioortizmanero]

This has been superseded by #199, so closing the PR.