Skip to content
This repository was archived by the owner on Feb 6, 2021. It is now read-only.

Conversation

DanDevDan
Copy link
Collaborator

Fixed undefined algo (wrong variable used). Converter command is now fully operational with hashlib algorithms.

@DanDevDan DanDevDan requested a review from a team June 18, 2020 22:51
@ItsDrike ItsDrike added priority: 2 - normal Normal priority status: needs review PR Is waiting to be reviewed type: bug Something isn't working labels Jun 19, 2020
Copy link
Contributor

@ItsDrike ItsDrike 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 why you renamed cogs/custom.py and adjusted master.py here, those features were supposed to go to #25, now we either have to wait for #25 to be merged or revert changes to these files and only keep the modifications to cogs/converters.py.

Copy link
Contributor

@ItsDrike ItsDrike left a comment

Choose a reason for hiding this comment

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

You need to pass all of Travis-CI checks before merging, if you don't know what's wrong, you can click details link next to the check which will point you to the specific check where you can see what's wrong, in this case the url for that is: this

I've pointed out multiple of these errors in my requested changes too, since they have to do with unnecessary line splitting. You should also use pre-commit so that you can run these checks locally before pushing here, for more info about that you should read through our CONTRIBUTING.md pre-commit is described in third rule there.

Comment on lines +26 to +27
embed = Embed(
title="Info", description=f":ping_pong: Pong! ({duration}ms)", color=Color.blurple(),)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
embed = Embed(
title="Info", description=f":ping_pong: Pong! ({duration}ms)", color=Color.blurple(),)
embed = Embed(title="Info", description=f":ping_pong: Pong! ({duration}ms)", color=Color.blurple())

Our line limit is 150, which this line is well within, there is no reason to split it.
Same thing applies to lines: 44-45, 75-76, 89-90

Comment on lines +17 to +18
client = commands.Bot(commands.when_mentioned_or(
PREFIX), case_insensitivity=True, owner_id=688275913535914014)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
client = commands.Bot(commands.when_mentioned_or(
PREFIX), case_insensitivity=True, owner_id=688275913535914014)
client = commands.Bot(commands.when_mentioned_or(PREFIX), case_insensitivity=True, owner_id=688275913535914014)

Our line limit is 150, which this line is well within, there is no reason to split it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I literally have no idea why that has happened.

Comment on lines +14 to +15
SUPPORT_SERVER = "https://discord.gg/CgH6Sj6"
INVITE = "https://discord.com/api/oauth2/authorize?client_id=715545167649570977&permissions=980675863&scope=bot"
Copy link
Contributor

Choose a reason for hiding this comment

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

These things are defined in constants.py it would be wiser to use that instead of re-defining them here.

https://github.com/The-Codin-Hole/HotWired-Bot/blob/6e776c3b99088e27767ae2bafaa1f41d9bc93a23/cogs/utils/constants.py#L18-L28

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I haven't actually made that change. No idea why it shows up.

@ItsDrike ItsDrike added status: stale Has had no activity for a while status: waiting for author Waiting for author to address a review or respond to a comment and removed status: needs review PR Is waiting to be reviewed labels Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

priority: 2 - normal Normal priority status: stale Has had no activity for a while status: waiting for author Waiting for author to address a review or respond to a comment type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants