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

Implement Vk Music Source Manager #207

Merged
merged 23 commits into from
Sep 15, 2024
Merged

Conversation

Krispeckt
Copy link
Contributor

No description provided.

@Krispeckt Krispeckt marked this pull request as draft July 27, 2024 21:10
@Krispeckt Krispeckt marked this pull request as ready for review August 1, 2024 20:18
@Krispeckt
Copy link
Contributor Author

I hope for a fairly quick review of this pr.
If you need it, here's the documentation for the music api https://vodka2.github.io/vk-audio-token/
The work done is not small, there were a lot of tests, all the bugs that were found by users are fixed.

@topi314
Copy link
Owner

topi314 commented Aug 12, 2024

since this pr seems to be active again, how long should I wait before reviewing it?

@Krispeckt
Copy link
Contributor Author

Idk 😅 but for me personally I'm all set, it's just that I've been informed that the lyric in the vk may still give it with timestamps

@topi314
Copy link
Owner

topi314 commented Aug 20, 2024

you want to rebase this pr onto master, there have been some readme changes

Copy link
Owner

@topi314 topi314 left a comment

Choose a reason for hiding this comment

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

can you also move all your stuff to the bottom and not inbetween existing audio sources?
notably documentation (README) & config

@topi314 topi314 changed the title Implement Vk Music Source Manager 😎 Implement Vk Music Source Manager Aug 20, 2024
@Krispeckt Krispeckt requested a review from topi314 August 21, 2024 11:52
@topi314 topi314 self-assigned this Sep 6, 2024
Copy link
Owner

@topi314 topi314 left a comment

Choose a reason for hiding this comment

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

please resolve conversations when you fixed what I asked there, else I need to check every single one of them myself.

not sure where those spaces come from but there is a .editorconfig which should let you simply format lavasrc.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Owner

@topi314 topi314 left a comment

Choose a reason for hiding this comment

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

i think this finally looks acceptable to me

@topi314 topi314 merged commit b2f23ce into topi314:master Sep 15, 2024
@topi314
Copy link
Owner

topi314 commented Sep 15, 2024

thanks

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.

2 participants