mcb-dev / mCodingBot

The Discord bot for the mCoding Discord server.
https://mcoding.io/discord
MIT License
12 stars 4 forks source link

Add Pep validation and name to output #48

Closed Lunarmagpie closed 2 years ago

Lunarmagpie commented 2 years ago

This is most definitely draft quality code.

image

Lunarmagpie commented 2 years ago

I think I will store the PepManager on the plugin Guess I need another subclass :trollface:

Lunarmagpie commented 2 years ago

I would remove the whole thing about embeds/no embeds, since we can now get more useful information. Also if we put the result inside an embed, we can use masked links so the result is less ugly.

I think you're right tbh Cc @trag1c

trag1c commented 2 years ago

I would remove the whole thing about embeds/no embeds, since we can now get more useful information. Also if we put the result inside an embed, we can use masked links so the result is less ugly.

I think you're right tbh Cc @trag1c

:+1:

trag1c commented 2 years ago

I also agree with trag1c's comment

Pep -> PEP PepManager -> PEPManager

You don't actually have to agree since Pep and PepManager don't follow PEP8 lol

CircuitSacul commented 2 years ago

Also, since we now have access to more detailed information, I think that all responses should be embedded. The Pep class could have two methods: __str__: Returns the one-line information, using a masked link (instead of putting the links inside parenthesis) embed: Returns a detailed embed of the Pep

Then auto PEP replies would return a single embed containing a line for each individual pep, and the command can return the detailed embed.

CircuitSacul commented 2 years ago

You don't actually have to agree since Pep and PepManager don't follow PEP8 lol

In that case ig they should be left as-is probably.

trag1c commented 2 years ago

You don't actually have to agree since Pep and PepManager don't follow PEP8 lol

In that case ig they should be left as-is probably.

what? Pep and PepManager DON'T follow PEP8

CircuitSacul commented 2 years ago

what? Pep and PepManager DON'T follow PEP8

Then why did you say I didn't have to agree lol. Go with whatever follows pep8

trag1c commented 2 years ago

what? Pep and PepManager DON'T follow PEP8

Then why did you say I didn't have to agree lol. Go with whatever follows pep8

because it doesn't matter whether you agree or not, renaming it is sort of a requirement in this case

Lunarmagpie commented 2 years ago

I agree with @trag1c

Lunarmagpie commented 2 years ago

Also, since we now have access to more detailed information, I think that all responses ...

I am against this because of how bulky embeds are. The button takes up enough space already.

Lunarmagpie commented 2 years ago

I agree with @trag1c

Changed my mind. The Pep class looks like constant when its capital and I dont like that tbh.

All variable references of pep follow pep8, so I think we should also follow that for classes. If both are changed then a lot of variables in this project would read as a constant at first glance; even though the color is different.

trag1c commented 2 years ago

I agree with @trag1c

Changed my mind. The Pep class looks like constant when its capital and I dont like that tbh.

Dude that's like naming your functions with camelCase???

Enderchief commented 2 years ago

For the class renaming discussion,

instead of Pep or PEP what about something like PEPObject or PEPItem?

Lunarmagpie commented 2 years ago

Dude that's like naming your functions with camelCase???

I don't understand what you want me to do

trag1c commented 2 years ago

Dude that's like naming your functions with camelCase???

I don't understand what you want me to do

"Rename Pep and PepManager to PEP and PEPManager"

trag1c commented 2 years ago

Dude that's like naming your functions with camelCase???

I don't understand what you want me to do

"Rename Pep and PepManager to PEP and PEPManager"

If you don't like PEP looking like a constant then pick a different name as suggested by @Endercheif

CircuitSacul commented 2 years ago

Also, since we now have access to more detailed information, I think that all responses ...

I am against this because of how bulky embeds are. The button takes up enough space already.

Screen Shot 2022-09-13 at 7 07 45 PM

I think it looks great.

Lunarmagpie commented 2 years ago

I think it looks great.

yeah its better than i though actually

CircuitSacul commented 2 years ago

yeah its better than i though actually

Should I just push my changes

Lunarmagpie commented 2 years ago

yes

Lunarmagpie commented 2 years ago

me

Enderchief commented 2 years ago

wait for ci to pass

Enderchief commented 2 years ago

:why: edit: gh cache skill issue