Twentysix26 / Red-Docs

Docs for Red - Discord bot
8 stars 16 forks source link

beatrice-witchcogs application #34

Closed PookaMustard closed 7 years ago

PookaMustard commented 8 years ago

List: beta Repository name: beatrice-witchcogs Author: PookaMustard Description: General-purpose cogs made for Red-DiscordBot, mostly search cogs.

Additional notes: The Bing cog uses code from Mash's IMDb cog, namely the JSON code. The same will be of the MyAnimeList cog later on, as it will take the same makeover. Mash has allowed this.

Eslyium commented 8 years ago
  1. Please Fix Your Info.jsons http://twentysix26.github.io/Red-Docs/red_cog_info_json/
  2. Not have features that are already in Red (nineball) Im not to sure on this one.. Ill leave this up to Twenty.
  3. People who worked on a cog must be properly credited If most of the JSON work was done by someone else id at least put them in the Info.json, Unless they said it was fine.

Other than that. Nice cogs.

PookaMustard commented 8 years ago
  1. Not sure what needs to be fixed about the info.jsons because I copied the formatting from other cogs. That said, I will copy the format from here.
  2. I can remove the nineball cog no problem.
  3. Noted that, will credit Mash in the Bing info.json.

Will get to work on fixing these soon.

PookaMustard commented 8 years ago

Took the following actions:

  1. Updated cog info.jsons to reflect the standard used by Red-Docs, also added a repo info.json
  2. Removed nineball cog
  3. Properly credited Mash (his name, his origin cog, and his permission) in the Bing info.json in the Description section.
Redjumpman commented 8 years ago

Your readme should have a contact section that provides info on how to contact you for issues with any of your cogs. Preferably a way to DM you in discord or the Red server to possibly filter help requests out of #support channel.

PookaMustard commented 8 years ago

That's complete as well. Readme now suggests either leaving an issue on my repo or sending me a message on Discord.

Twentysix26 commented 8 years ago

You're not specifying 3rd party libs requirements in your info.json files. There are no checks at load to let the user know they're missing them either.

https://github.com/PookaMustard/beatrice-witchcogs/blob/0f486bfc51cfc383899348b84d3936a9b6f2196e/commandrequest/commandrequest.py#L77 This is a huge security hole that lets everyone run arbitrary code. You also expect the user to edit the code to configure it. Huge no no.

Classes names should be CamelCase. This is the norm and what the user expects when trying to do [p]help CogName.

That was just a quick look, it's overall very messy, lots of useless imports / code that derives from copy pasting and some commands of dubious utility.

I also suggest to take a good read on how to use git properly, from the commits you seem to be using github as an online text editor.

PookaMustard commented 8 years ago

All your points taken for granted, but I'm comfortable with the way I use github and as such, I will continue to use it as an online text editor. The functionality is provided for me and I see no reason why to deviate from my current setup.

PookaMustard commented 8 years ago

Remade Echo and Command Request. For both, simpler (bot-provided) code is used rather than arbitrary code, and for the latter, it now uses a homegrown basic JSON file system for saving the channel ID as opposed to editing the cog itself. Other code updates might be on the way.

Also, using the CamelCase class naming.

By the way, what are those "commands of dubious utility"?

Twentysix26 commented 8 years ago

What's the purpose of this command? https://github.com/PookaMustard/beatrice-witchcogs/blob/ae30e2b1adc5017902f214f44403103b3c3458bc/commandrequest/commandrequest.py#L13

PookaMustard commented 8 years ago

For bots with owners that accept various custom command suggestions from a large community, [p]commandrequest comes in handy. What it does is send these suggestions from separate channels to just one. For example, there are four servers, A, B, C and X. Rather than searching for command suggestions in servers A, B, and C (including its channels), all the suggestions get sent to X, and in one channel.

That is its purpose, and I will make it clearer in the info.json. Speaking of coding, I just improved the overall coding of the Bing cog.

Redjumpman commented 8 years ago

For your cogs that require 3rd party libraries, you need to specify to the user that it's required, and give them an error if it's not loaded. An example may look like this:

In your imports

try:   # check if BeautifulSoup4 is installed
    from bs4 import BeautifulSoup
    soupAvailable = True
except:
    soupAvailable = False

In your setup

def setup(bot):
    if soupAvailable:
        n = Cogname(bot)
        bot.add_cog(n)
    else:
        raise RuntimeError("You need to run 'pip3 install beautifulsoup4'")

As 26 Mentioned, you should also specify in your info.JSON file any required 3rd party libraries. But the above helps catch those that don't read and points them in the right direction.

PookaMustard commented 8 years ago

Good catch, I like this other way to do it. Will set it up immediately.

Redjumpman commented 8 years ago

Here is small list of stuff I caught on some of your cogs. Please don't take this the wrong way, just trying to up the quality and help out with your request.

Appsearcher

Remove the following from your import list:

Unused variables:

Bing

Command Request

Echo

Isitdown

animelist

Pokemon

ThePirateBay

Don't see this as an attack. I'm here to help.

PookaMustard commented 8 years ago

Thanks for the list, Redjumpman! This should be a good enough serving point to know which problems I really need to tackle when handling Python and Red.

The thing about import discord is because it was in the original documentation for how to build a cog. As for appsearcher which is the biggest list, that has to wait as it may go through a makeover, similar to Bing. And those indentations are killing the hell out of me (the mixing that is), so I will also see it.

I'll have to take a break from doing these fixes for a short while. I should resolve some of them if not all by tomorrow.

Chovin commented 8 years ago

I recommend hanging out in #coding and #testing to learn more about the current best practices when writing cogs. This will also help make sure your cogs don't interfere with other cogs in the bot and that you don't have any security issues.

The how to build a cog guide is not necessarily the best and it won't teach you everything. Keep at it 👍

PookaMustard commented 8 years ago

I just knew of autopep8 and flake8, which will make sorting the code even easier. I pushed a commit to Bing which sorts all the listed issues. Will look after the rest.

PookaMustard commented 8 years ago

Did the same as above for some of the remaining cogs. Also added a dictionary cog which uses the PyDictionary module, which put what I learned into perspective in a new utility.

Eslyium commented 7 years ago

Closed, Sorry for the long delay,

Finally had time to come back and look at all of these.

Feel free if you become active again to retry.