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

Added 8ball command #176

Merged
merged 6 commits into from Feb 22, 2022
Merged

Added 8ball command #176

merged 6 commits into from Feb 22, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jan 27, 2022

Added an 8ball command

Copy link
Member

@vivekashok1221 vivekashok1221 left a comment

Choose a reason for hiding this comment

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

Few nitpicks.
And please open an issue and get it approved next time.

bot/exts/fun/magic_8ball.py Outdated Show resolved Hide resolved
bot/exts/fun/magic_8ball.py Outdated Show resolved Hide resolved
bot/exts/fun/magic_8ball.py Outdated Show resolved Hide resolved
Luka and others added 2 commits January 28, 2022 08:03
Co-authored-by: Vivek Ashok <vivekashok1221@gmail.com>
Co-authored-by: Vivek Ashok <vivekashok1221@gmail.com>
@ghost
Copy link
Author

ghost commented Jan 28, 2022

I will commit these changes locally. I thought this came under gurkbot#166

Sorry.

@ghost
Copy link
Author

ghost commented Jan 28, 2022

Am not sure if I should move replies into
https://github.com/gurkult/gurkbot/blob/main/bot/resources/bot_replies.yml
Would that be good practice to move them?

@ghost
Copy link
Author

ghost commented Jan 28, 2022

All done

Copy link
Member

@vivekashok1221 vivekashok1221 left a comment

Choose a reason for hiding this comment

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

Almost everything looks good. Minor change.

bot/exts/fun/magic_8ball.py Outdated Show resolved Hide resolved
Co-authored-by: Vivek Ashok <vivekashok1221@gmail.com>
@ghost
Copy link
Author

ghost commented Jan 29, 2022

Almost everything looks good. Minor change.

Done.

Copy link
Member

@vivekashok1221 vivekashok1221 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ghost
Copy link
Author

ghost commented Jan 29, 2022

My first LGTM. This will forever be remembered by me. Thank you, looking forward for it to get merged.

@CommunistDucky
Copy link

sup

@ghost
Copy link
Author

ghost commented Feb 4, 2022

Can we wrap this up quick? If you have changes to request, please do, since I will be busy in the upcoming days.

Copy link
Contributor

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

lgtm

@ghost
Copy link
Author

ghost commented Feb 22, 2022

Nice Nice thanks

@Akarys42 Akarys42 merged commit c251782 into gurkult:main Feb 22, 2022
@Akarys42
Copy link
Member

Merge has been reverted due to a Git history problem. Please use branches when making pull requests.

I'd also like to see an issue before the next PR.

@ghost ghost mentioned this pull request Feb 22, 2022
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.

None yet

4 participants