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.

Sign up for GitHub

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

Jump to bottom

Create Lyrics.js #221

Merged
merged 6 commits into from Nov 12, 2022
Merged

Create Lyrics.js #221

merged 6 commits into from Nov 12, 2022

Conversation

berusvn
Copy link
Contributor

@berusvn berusvn commented Sep 7, 2022

No description provided.

@lorypelli
Copy link

Who are you?

@meqativ
Copy link
Contributor

meqativ commented Sep 7, 2022 •
edited

Who are you?

they're berusvn

@3eif
Copy link
Owner

3eif commented Sep 7, 2022

true..

@3eif 3eif changed the base branch from 2.4.0 to master September 7, 2022 14:27
@3eif 3eif changed the base branch from master to 2.5.0 September 7, 2022 14:33
3eif
3eif requested changes Sep 7, 2022
View reviewed changes
Copy link
Owner

@3eif 3eif left a comment •
edited

Choose a reason for hiding this comment

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

ezgif-1-1739e21625

The embed doesn't always properly update when the buttons are pressed.

@3eif 3eif linked an issue Sep 7, 2022 that may be closed by this pull request
Lyrics Command #208
Closed
@lorypelli
Copy link

lorypelli commented Sep 7, 2022 •
edited

Please make that it will use song history for options as the play command also you should make context menu for this

@berusvn
Copy link
Contributor Author

berusvn commented Sep 7, 2022

idk, i just apply paginate from messagehelper

@lorypelli
Copy link

idk, i just apply paginate from messagehelper

I was talking with tetracyl

berusvn
berusvn commented Sep 8, 2022
View reviewed changes
Copy link
Contributor Author

@berusvn berusvn left a comment

Choose a reason for hiding this comment

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

maybe we just fix paginate from msghelper

@3eif
Copy link
Owner

3eif commented Sep 8, 2022

There is nothing wrong with the paginate function since it works for shards command and the queue command.

@berusvn
Copy link
Contributor Author

berusvn commented Sep 8, 2022

There is nothing wrong with the paginate function since it works for shards command and the queue command.

hmm, maybe i should recheck the code

@berusvn
Update Lyrics.js
ef4b6e5 
Fix paginate and replace lyrics-finder with genius-lyrics
@berusvn berusvn requested a review from 3eif September 8, 2022 19:41
3eif
3eif requested changes Sep 9, 2022
View reviewed changes
Copy link
Owner

@3eif 3eif left a comment •
edited

Choose a reason for hiding this comment

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

The lyrics variable is never defined. Please test your code before pushing another commit.

@berusvn
Update Lyrics.js
e146e24 
when I deleted console.log I deleted the variable assignment
@berusvn
Copy link
Contributor Author

berusvn commented Sep 9, 2022

The lyrics variable is never defined. Please test your code before pushing another commit.

when I deleted console.log I deleted the variable assignment sr about that @@

@berusvn berusvn requested a review from 3eif September 9, 2022 16:20
@3eif
Copy link
Owner

3eif commented Sep 17, 2022

Thanks, will test again when I have time

@3eif 3eif merged commit 4350a96 into 3eif:2.5.0 Nov 12, 2022
@3eif
Copy link
Owner

3eif commented Nov 12, 2022

Thanks for the pr @berusvn

@3eif 3eif mentioned this pull request Nov 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@3eif 3eif 3eif approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Lyrics Command
4 participants
@berusvn @lorypelli @meqativ @3eif