Skip to content

Conversation

@RahwaZeslusHaile
Copy link

Manchester | 25-SDC-Nov | Rahwa Haile | Sprint 4 | Implement cowsay in Python

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

Mimics the basic behavior of the cowsay command line tool in Python.
Supports a message input and optional --animal selection.
Shows an error for unsupported animals, similar to the official CLI.

Questions

The implementation currently mimics the official cowsay CLI by showing an error when an unsupported animal is provided, using a hardcoded list of animals. Is this approach acceptable, or would you recommend a different method for handling unsupported animals? Thank you!

@RahwaZeslusHaile RahwaZeslusHaile added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 2, 2025
@LonMcGregor LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Dec 23, 2025
@LonMcGregor
Copy link

I think you have 2PRs here. Could you close one and let me know which one you want reviewed?

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Dec 23, 2025
@RahwaZeslusHaile
Copy link
Author

@LonMcGregor, sorry for the confusion. I’ve closed the other PR to avoid confusion and ensure all feedback is focused on this one. This is the PR I would like you to review. Thank you!

@RahwaZeslusHaile RahwaZeslusHaile added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 23, 2025
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

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

Good start on this task, I've left some pointers

@@ -1 +1,2 @@
node_modules
.venv

Choose a reason for hiding this comment

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

Correct, you want to exclude the .venv directory. But what about the things you are installing into the .venv. Is there enough comitted here for me to be able to re-create your venv on my machine? which file is missing?

text = " ".join(args.message)
animal = args.animal

supported_animals = [

Choose a reason for hiding this comment

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

Part of the task spec is finding a way to write this without you having to declare this array yourself. can you figure out how to get the animals directly from the library?

@LonMcGregor LonMcGregor removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants