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

Add rustdoc to connect crate #1455

Merged
merged 12 commits into from
Feb 22, 2025
Merged

Conversation

photovoltex
Copy link
Member

@photovoltex photovoltex commented Jan 21, 2025

I think I initially tried to enforce rustdoc on the whole project, but that just is in insane task at this stage.

Well anyhow... I got around to add rustdoc for the connect crate and additionally a cleanup of the exposed items, so that only the necessary are exposed.

I also had to change up the CHANGELOG.md as some previous entries where are out of that with the cleanup changes.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 11 changed files in this pull request and generated 1 comment.

Files not reviewed (6)
  • CHANGELOG.md: Evaluated as low risk
  • connect/README.md: Evaluated as low risk
  • connect/src/lib.rs: Evaluated as low risk
  • connect/src/model.rs: Evaluated as low risk
  • connect/src/state/metadata.rs: Evaluated as low risk
  • src/main.rs: Evaluated as low risk

Copy link
Member

@roderickvd roderickvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite the effort to get everything documented!

@roderickvd roderickvd assigned photovoltex and unassigned roderickvd Jan 22, 2025
@roderickvd
Copy link
Member

When you're looking for anyone's review, you can specifically request / add them as reviewers and keep the assignee as yourself.

@photovoltex
Copy link
Member Author

When you're looking for anyone's review, you can specifically request / add them as reviewers and keep the assignee as yourself.

Okay will do! You just said in another message that I should assigning you, but I suppose I mistook assigning to the PR with review assigning.

@photovoltex
Copy link
Member Author

Oh and big thanks for the review @roderickvd :D

@photovoltex photovoltex force-pushed the connect-docs branch 2 times, most recently from fd6de96 to bed00a6 Compare January 23, 2025 20:56
@roderickvd roderickvd requested a review from Copilot January 28, 2025 20:13
@photovoltex
Copy link
Member Author

When there aren't any points left to address, I will merge it sometime the following days.

@photovoltex
Copy link
Member Author

Will merge this after #1462 is merged so that I don't create more conflicts then necessary

@photovoltex
Copy link
Member Author

photovoltex commented Feb 22, 2025

If someone want's to take a look over the rebased changes, feel free. Will wait until the evening (around 4-8h from now) and then merge it.

After that there is just a single thing I still want to "add" or rather "re-add" in the current version. During the dealer refactoring I removed the ability to to start playback of tracks. That also breaks the option to start uris via the web-api. I have the code ready to merge, and will just wait for this PR to be merged, as the changes are based on this branch.

@photovoltex photovoltex merged commit 09b4aa4 into librespot-org:dev Feb 22, 2025
13 checks passed
@photovoltex photovoltex deleted the connect-docs branch February 22, 2025 22:39
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