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

Convert to Slash Commands #7

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Convert to Slash Commands #7

wants to merge 7 commits into from

Conversation

janine9vn
Copy link
Contributor

@janine9vn janine9vn commented Nov 25, 2020

It's been awhile and this now looks drastically different!

BLUF

This PR completely removes the !join_ag !join_wg` commands. Instead, it implements slash command functionality.
Slash Commands provide a better user experience and are easier to parse with the setup.

There are many changes to allow this to happen but most of them involve replacing functionality 1:1 or removing code that is no longer needed.

Docker

This PR also introduces Docker. Lovelace is currently running in an Azure Container Instance. Once this PR is merged and the Dockerfile gets added to the repository, I can begin looking at setting up CI/CD with Azure Containers.

- constants moved to constants.py file
- create an overall "bot" directory
- renamed lovelace.py to bot.py
Lovelace is now a package! It requires a different command to run,
the README is updated.
There is now an overaching bot directory folder with appropriate
__init__ and __main__ files.
Additionally, there are two new folder strucutres: exts and utils.
exts is for Extensions while util is for utility functions and classes.
Functionality for autoloading extensions was also added. It checks if
a setup function is in the file, if yes than it'll
load in the extension.
@janine9vn janine9vn requested a review from twavv November 25, 2020 03:07
twavv
twavv previously approved these changes Nov 25, 2020
Copy link
Contributor

@twavv twavv left a comment

Choose a reason for hiding this comment

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

Nothing blocking, but I left my opinions inline.

I think you should address the feedback, but if you merge without doing so, I won't be offended. Especially since the project is still relatively small.

bot/__main__.py Outdated
from bot import exts
from bot.bot import bot

if __name__ == "__main__":
Copy link
Contributor

Choose a reason for hiding this comment

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

Truthfully, I think this is a bit too "cute" and magical. It would be better™ to just import the packages and then call .setup() on them, or have a "extensions" array (and then iterate over them and call .setup() on them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I purposely went with a more "magical" method because I didn't want to have an extensions array we'd have to adjust and change with each extension added. Do you think it'd be better to have call out the different extensions and load them individually? @travigd

Copy link
Contributor

Choose a reason for hiding this comment

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

It's prooooooobably fine. Java land does this a bunch with their auto-wired beans.

That being said, I do think it's better to just have an array where you specify all of them explicitly.

bot/utils/role_checks.py Outdated Show resolved Hide resolved
bot/exts/affinity_working_groups.py Outdated Show resolved Hide resolved
bot/bot.py Outdated Show resolved Hide resolved
janine9vn and others added 5 commits November 24, 2020 22:39
Co-authored-by: Travis DePrato <[email protected]>
I've simplified the bot set-up and made it not a module
to make sure that setting it up locally for contributions is easier.

The other big change is the less-smart, but more-readable/understandable
method of loading extensions. If a new extenion is added it must be
manually added to Lovelace's `__init__()`.

These changes are also made in preperation for adding in slash commands.
This adds in the Dockerfile to run a basic instance of this bot.
It takes the bot Token as an ENV argument. Eventually this should be
changed to use a proper secrets file / vault.
This probably-too-large commit is a complete overhaul of the
affinity and working group commands to use Slash Commands
instead.

Overall, the previous `!` commands were removed and instead replaced
with the slash commands response.

It makes some assumptions about the server setup and reduces some of the
code complexity and constants required.

With this includes a file to register and update the slash commands.
With the bot simplication, this removes files that are no longer used.
@janine9vn janine9vn requested a review from twavv June 16, 2021 21:35
@janine9vn janine9vn changed the title Extensions Refactor Convert to Slash Commands Jun 16, 2021
@janine9vn janine9vn dismissed twavv’s stale review June 16, 2021 21:54

Substantial changes, this PR should be re-reviewed

Copy link
Contributor

@twavv twavv left a comment

Choose a reason for hiding this comment

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

this still shows up as a pending review for me in gh that i want to get rid of,,, so,,, here you go! 😅

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