Cowsay by Kronifer · Pull Request #699 · python-discord/sir-lancebot

Kronifer · 2021-04-20T21:37:37Z

Relevant Issues

Closes #667

Description

Added Cowsay command using the cowsay module

Reasoning

I used a pre-existing module, because a lot of the work had already really been done for me that way

Screenshots

Additional Details

ASCII art breaks on mobile for some reason

Did you:

Join the Python Discord Community?
If dependencies have been added or updated, run pipenv lock?
Lint your code (pipenv run lint)?
Set the PR to allow edits from contributors?

doublevcodes

One pointer:

What happens if the user enters a character that isn't supported? Add some sort of error for this.

Co-authored-by: Vivaan Verma <54081925+doublevcodes@users.noreply.github.com>

ToxicKidz

Thanks for the PR! This looks pretty good to me, just one comment. Will test this later.

Kronifer · 2021-04-21T00:28:56Z

@doublevcodes resolved the errors

janine9vn

Thanks for the PR! I have some minor recommendations to make this command a bit more robust to user input. Otherwise the command is fun and simple, so the rest of my review will focus on the overall contribution process and things to keep in mind for the future.

Commit messages!
Commit messages are hugely important in the review process and also looking back if we're curious why a command was coded a specific way. I'd recommend adding reasoning as to why you're add/removing/changing how something works.

For instance you have a commit that remove a for loop. I don't know why it was there originally or why it was removed. Having that information would be useful. It would also be useful for the commit where you removed trex from the documentation. I'm not sure why you removed it from the documentation although I imagine it has something to do with the fact that it's very likely to generate a response too large for discord to post.

Shivansh-007

Could you explain why you removed Trex from the documentation in commit 2a58874.

Co-authored-by: Shivansh-007 <shivansh-007@outlook.com>

…o cowsay

Kronifer · 2021-05-13T14:16:39Z

@janine9vn ive resolved the changes you requested.

Shivansh-007

Alright, few more nitpicks

Xithrius · 2021-07-31T07:21:05Z

@Kronifer what's your status for this PR?

brad90four

A small question / suggestion for sending the user the character that raised the un-supported character error.

Kronifer · 2021-08-12T23:07:40Z

@Xithrius so sorry, I've been caught up in stuff but I'll be getting back to this shortly

Co-authored-by: Shivansh-007 <shivansh-007@outlook.com>

Co-authored-by: kgolawski <konrad.golawski@gmail.com>

Co-authored-by: brad90four <42116429+brad90four@users.noreply.github.com>

Xithrius · 2021-08-12T23:26:58Z

@Xithrius so sorry, I've been caught up in stuff but I'll be getting back to this shortly

No problem. Remember to solve the conflicts in poetry.lock!

Kronifer · 2021-08-17T15:41:50Z

Alright, resolved that

Xithrius · 2021-08-17T19:29:54Z

There seems to be some linting errors as well. Run poetry run task lint to see what they are on your end in the repository, and run poetry run task precommit to make sure everything is correct before anything is committed.

ventaquil

Generally looks fine for me.

Kronifer · 2021-08-24T17:47:28Z

@ToxicKidz please re-review

Akarys42

Hey @Kronifer, first thanks a lot for all the time you are putting into this PR.

It has come to our attention that the package you added, cowsay, is licensed under GPL, which isn't compatible with the MIT license. We need to remove that package and replace it with something else.

One solution could be to copy the animals from an R clone which is licensed under MIT. We will let you solve this the way you want!

Kronifer · 2021-09-08T14:22:46Z

I've returned to school, so this may take longer than anticipated. I'll try and get it done soon!

Kronifer · 2021-09-13T14:20:37Z

I'm putting this on pause until I have .quack done

Xithrius · 2021-10-07T18:54:33Z

I'm putting this on pause until I have .quack done

@Kronifer .quack has been merged. Can you finish up this PR? Also, there seems to be a singular file conflict with poetry.lock. Please fix that up.

Kronifer · 2021-10-07T18:55:46Z

@Xithrius thanks for reminding me that this exists, I'll get on it soon

Kronifer and others added 4 commits March 2, 2021 17:57

Update Pipfile

fc8fae8

Merge remote-tracking branch 'upstream/main'

b2d3d21

Merge remote-tracking branch 'upstream/main'

a5a0f3e

Added cowsay command

723a22e

Kronifer requested a review from Akarys42 as a code owner April 20, 2021 21:37

doublevcodes suggested changes Apr 20, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Kronifer and others added 5 commits April 20, 2021 16:54

import only one part of cowsay module

e13e4e5

Co-authored-by: Vivaan Verma <54081925+doublevcodes@users.noreply.github.com>

Changed to fit single import

701c13a

Co-authored-by: Vivaan Verma <54081925+doublevcodes@users.noreply.github.com>

Removed loop

35e349f

Removed loop

b107d93

Locked and added error

fda9541

ToxicKidz suggested changes Apr 20, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Removed trex from documentation

2a58874

janine9vn requested changes Apr 21, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Shivansh-007 suggested changes Apr 21, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Kronifer and others added 5 commits April 21, 2021 07:34

Added spacing

44635c7

Co-authored-by: Shivansh-007 <shivansh-007@outlook.com>

Use char_names.join() for docs

7cb229a

Merge branch 'cowsay' of https://github.com/Kronifer/sir-lancebot int…

95d664d

…o cowsay

Modified cowsay docs to not return an error

34183c6

Fixed import order

b1ed3e3

Xithrius added area: backend Related to internal functionality and utilities season: evergreen status: needs review Author is waiting for someone to review and approve type: feature Relating to the functionality of the application. labels Apr 22, 2021

Shivansh-007 suggested changes Apr 22, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

Xithrius added status: waiting for author Waiting for author to address a review or respond to a comment and removed status: needs review Author is waiting for someone to review and approve labels Apr 24, 2021

Fixed problem shivansh had

930c594

Shivansh-007 reviewed May 20, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Comment thread bot/exts/evergreen/cowsay.py Outdated

brad90four reviewed Aug 11, 2021

View reviewed changes

Comment thread bot/exts/evergreen/cowsay.py Outdated

Kronifer and others added 4 commits August 12, 2021 18:08

Removed bot from cowsay call

a34f64b

Co-authored-by: Shivansh-007 <shivansh-007@outlook.com>

Small change

450ae37

Co-authored-by: Shivansh-007 <shivansh-007@outlook.com>

Update bot/exts/evergreen/cowsay.py

d8d71c6

Co-authored-by: kgolawski <konrad.golawski@gmail.com>

Small error change given by brad90four

ae85a09

Co-authored-by: brad90four <42116429+brad90four@users.noreply.github.com>

Merge branch 'main' into cowsay

b73aee2

Kronifer requested a review from ToxicKidz August 17, 2021 15:41

fixed lint

24b2773

ventaquil reviewed Aug 19, 2021

View reviewed changes

Akarys42 suggested changes Aug 26, 2021

View reviewed changes

wookie184 added status: waiting for author Waiting for author to address a review or respond to a comment and removed status: needs review Author is waiting for someone to review and approve labels Aug 30, 2021

Xithrius added category: fun Related to fun and games and removed category: evergreen labels Sep 6, 2021

Xithrius added status: needs review Author is waiting for someone to review and approve and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Oct 7, 2021

Kronifer closed this Oct 8, 2021

Kronifer deleted the cowsay branch October 8, 2021 18:40

Xithrius removed the status: needs review Author is waiting for someone to review and approve label Nov 10, 2021

Uh oh!

Conversation

Kronifer commented Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant Issues

Description

Reasoning

Screenshots

Additional Details

Did you:

Uh oh!

doublevcodes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ToxicKidz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kronifer commented Apr 21, 2021

Uh oh!

janine9vn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Shivansh-007 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kronifer commented May 13, 2021

Uh oh!

Shivansh-007 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Xithrius commented Jul 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brad90four left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kronifer commented Aug 12, 2021

Uh oh!

Xithrius commented Aug 12, 2021

Uh oh!

Kronifer commented Aug 17, 2021

Uh oh!

Xithrius commented Aug 17, 2021

Uh oh!

ventaquil left a comment

Choose a reason for hiding this comment

Uh oh!

Kronifer commented Aug 24, 2021

Uh oh!

Akarys42 left a comment

Choose a reason for hiding this comment

Uh oh!

Kronifer commented Sep 8, 2021

Uh oh!

Kronifer commented Sep 13, 2021

Uh oh!

Xithrius commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kronifer commented Oct 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Kronifer commented Apr 20, 2021 •

edited

Loading

Xithrius commented Jul 31, 2021 •

edited

Loading

Xithrius commented Oct 7, 2021 •

edited

Loading