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

Set up some simple Discord bot boilerplate #2

Merged
merged 6 commits into from
Oct 3, 2018
Merged

Set up some simple Discord bot boilerplate #2

merged 6 commits into from
Oct 3, 2018

Conversation

kvcvc
Copy link
Contributor

@kvcvc kvcvc commented Oct 2, 2018

Pull request for issue #1. Created the basic Discord Bot structure and added some flake8 guidelines as I know you guys love them!

Copy link
Member

@lemonsaurus lemonsaurus left a comment

Choose a reason for hiding this comment

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

I'm not sure if we should use Pipenv or these requirements txt files. This is a good PR overall, but there are changes that are needed, and I'd like if someone else could chime in on whether pipenv would be worth doing for this.

bot/bot.py Outdated Show resolved Hide resolved
bot/cogs/template.py Show resolved Hide resolved
bot/cogs/template.py Outdated Show resolved Hide resolved
bot/constants.py Outdated
@@ -0,0 +1 @@
token = ''
Copy link
Member

Choose a reason for hiding this comment

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

This is not going to work. We can't be committing the token into the source control, so we'll need to find a better solution for this. Perhaps an environment variable?

Copy link
Member

Choose a reason for hiding this comment

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

Also, for any constants we do need, perhaps dataclasses (or NamedTuples) would be a more organized option. Look at how we do it in Python for examples on how this might work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A temporary solution is a environ.get to get the bot token from a env. variable.

requirements-dev.txt Outdated Show resolved Hide resolved
@modelmat
Copy link

modelmat commented Oct 2, 2018

cc/@heavysaturn this isn't a project designed for many people to run and use, so locking everything verey heavily isn't really required. requirements. txt is also much simpler for beginners to use (and this repo is aimed at that).

additionally flake should probably be a dev req not a standard req

@GhostofGoes
Copy link
Contributor

Pipenv would be good to have, in addition to the requirements.txt. This is an application, not a library, and one with a fairly large set of dependencies. It's much easier for someone to just pipenv sync than have to create a virtual environment and then install the dependencies.

Copy link
Member

@lemonsaurus lemonsaurus left a comment

Choose a reason for hiding this comment

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

Getting warmer, but still got a bit to go before we can approve this.

As for the discussion above, I think pipenv is the way to go, but we can create a separate issue for it. We should implement pipenv, and also write some documentation in the wiki for this repo so people will know how to get up and running so they can write code for this project. I'll put both of those in an issue, and we can tackle that after this has been merged.

bot/cogs/template.py Outdated Show resolved Hide resolved
bot/cogs/template.py Outdated Show resolved Hide resolved
bot/cogs/template.py Outdated Show resolved Hide resolved
bot/bot.py Outdated
from traceback import print_exc
from os import environ


Copy link
Member

Choose a reason for hiding this comment

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

double empty lines here would not pass PEP8. have you run this code through flake8 at all?

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 just ran through flake8 and even tried online PEP8 tests, it doesn't appear to flag anything? Should this be 1 line?

bot/bot.py Outdated Show resolved Hide resolved
@lemonsaurus
Copy link
Member

Yep, that works for me. There's a weird newline but we can clean it up in the next PR.

Thanks, good job!

@lemonsaurus lemonsaurus merged commit 9a81988 into python-discord:master Oct 3, 2018
@scragly scragly added the area: backend Related to internal functionality and utilities label Feb 12, 2019
sco1 pushed a commit that referenced this pull request May 15, 2019
@kvcvc kvcvc changed the title Set up some simple Discord bot boilerplate. Set up some simple Discord bot boilerplate Jan 24, 2021
Xithrius pushed a commit that referenced this pull request May 16, 2021
Implement the math and science questions, added a dynamic_id checker
Xithrius pushed a commit that referenced this pull request Jul 31, 2021
@DMFriends DMFriends mentioned this pull request Sep 28, 2021
2 tasks
Xithrius added a commit that referenced this pull request Oct 22, 2021
* Add WTF Python Command

* Fix grammar in docstrings, remove redundant variable, remove the use of a wrapper

* Fix indentation issues and make use of triple quotes

* Update docstrings and remove redundant list()

* Change minimum certainty to 75.

* Make 'make_embed' function a non async function

* Try to unload WTFPython Extension if max fetch requests hit i.e. 3 else try to load the extension.

* Correct log messages.

* Make flake8 happy :D

* Remove redundant class attributes and async functions.

* Apply requested grammar and style changes.

* Fix unload and load extension logic.

* Fix typo in `WTF_PYTHON_RAW_URL`

* Changed fuzzy_wuzzy to rapidfuzz

Since rapidfuzz also has an extractOne method, this should be a
straight replacement with the import statement.

* Move wtf_python.py to bot/exts/utilities, flake8

Moved the file to the correct location after merge with main,
made changes from the last open suggestions from the previous PR,
had to make WTF lowercase to pass flake8 on lines 54 and 118.

* Fix trailing commas and long lines

* # This is a combination of 3 commits.
# This is the 1st commit message:

Squashing small commits

Small changes and fixes

-Added "the" to setup docstring
-Fixed typo for mis-matched WTF and wtf in get_wtf_python_readme
-Fixed ext location
-Added more information to fuzzy_match_header docstring regarding
 the MINIMUM_CERTAINTY and what the score / value represents.

Add wildcard to capture unused return

Updated MINIMUM_CERTAINTY to 75

Change MINIMUM_CERTAINTY to 50

Squash commits from Bluenix suggestions

Fix docstring for fuzzy_match_header

Swap if / else for match

Fix functools import

Rename get_wtf_python_readme to fetch_readme

Collapse self.headers into one line

Fix docstring for fuzzy_match_header

Swap if / else for match

# This is the commit message #2:

Fix functools import

# This is the commit message #3:

Rename get_wtf_python_readme to fetch_readme

* Squashing commits

Squashing small commits

Small changes and fixes

-Added "the" to setup docstring
-Fixed typo for mis-matched WTF and wtf in get_wtf_python_readme
-Fixed ext location
-Added more information to fuzzy_match_header docstring regarding
 the MINIMUM_CERTAINTY and what the score / value represents.

Add wildcard to capture unused return

Updated MINIMUM_CERTAINTY to 75

Change MINIMUM_CERTAINTY to 50

Squash commits from Bluenix suggestions

Fix docstring for fuzzy_match_header

Swap if / else for match

Fix functools import

Rename get_wtf_python_readme to fetch_readme

Collapse self.headers into one line

Fix docstring for fuzzy_match_header

Swap if / else for match

Fix functools import

Rename get_wtf_python_readme to fetch_readme

Collapse self.headers into one line

Fix type hints with dict

Add match comment for clarity

* Add debug logs, and send embed

* Add markdown file creation

Big change here is to create a .md file based on the matched header.
I save the raw text as a class attribute, then slice it based on the
index returned by the .find() method for the header, and the separator
"/n---/n".

* Move the list(map(str.strip , ...) to for loop

* Remove line

* Use StringIO for file creation

* Update file creation with StringIO

* Remove embed file preview

* chore: update wtf_python docstring

* chore: change regex to search, remove file preview

* feat: update caching as recommended

Minor fixes to import statements as well.

Co-authored-by: Bluenix2 <bluenixdev@gmail.com>

* chore: remove logging statements

* feat: scheduled task for fetch_readme

* chore: fix hyperlink, remove dead code

* fix: capitalization clean up

* chore: remove unused code

* chore: remove more unused code

* feat: add light grey logo image in embed

* feat: add light grey image

* chore: remove debug log message

* feat: add found search result header

* feat: limit user query to 50 characters

* cleanup: remove debug logging

* fix: restructure if not match statement

Co-authored-by: Bluenix <bluenixdev@gmail.com>

Co-authored-by: Shivansh-007 <Shivansh-007@users.noreply.github.com>
Co-authored-by: Shivansh-007 <shivansh-007@outlook.com>
Co-authored-by: Bluenix2 <bluenixdev@gmail.com>
Co-authored-by: Xithrius <15021300+Xithrius@users.noreply.github.com>
brad90four added a commit that referenced this pull request Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend Related to internal functionality and utilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants