Skip to content

TASK: Add smile method (#50)#55

Closed
kavyangaHA wants to merge 1 commit intoTheGittyPerson:mainfrom
kavyangaHA:feature/smile-method
Closed

TASK: Add smile method (#50)#55
kavyangaHA wants to merge 1 commit intoTheGittyPerson:mainfrom
kavyangaHA:feature/smile-method

Conversation

@kavyangaHA
Copy link
Copy Markdown

Summarize your changes here.
Closes #50

Changes

  • add a smile method
  • use emojis

Resolves #50

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening your first PR on this repository! Make sure you've read our contributing guidelines — it's very important to us.

@TheGittyPerson TheGittyPerson changed the title add smile method to person class (#50) TASK: Add smile method (#50) Mar 19, 2026
@TheGittyPerson TheGittyPerson added feature New feature or enhancement task This is a task issue / PR completing a task labels Mar 19, 2026
@TheGittyPerson
Copy link
Copy Markdown
Owner

Will be reviewing in an hour 👍

@TheGittyPerson TheGittyPerson self-requested a review March 19, 2026 05:46
Copy link
Copy Markdown
Owner

@TheGittyPerson TheGittyPerson left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! Just a few formatting fixes before we move on

Comment thread src/person.py
"""
print("\U0001f44b") # Unicode for waving hand emoji

def smile(self,smile_type:str = "😊") -> None:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since this is a static method, add @staticmethod before this line (and remove the self argument)

Comment thread src/person.py
"""
print("\U0001f44b") # Unicode for waving hand emoji

def smile(self,smile_type:str = "😊") -> None:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Also add a space after the comma and colon.

Comment thread src/person.py
Comment on lines +66 to +68
"""Prints a smile emoji.
Args: smile_type:The type of smile to display.
options: '🙂', '☺️', '😊', '😁', '😄' """
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is a bit weirdly formatted. Consider:

"""Print a smile emoji.

Args:
    smile_type: The type of smile to display.
                Options: '🙂', '☺️', '😊', '😁', '😄'
"""

Notice the docstring should also be in imperative mood instead of descriptive

@TheGittyPerson
Copy link
Copy Markdown
Owner

TheGittyPerson commented Mar 29, 2026

@kavyangaHA Hello! Here's a friendly reminder that it's been 10 days since my review of your PR. If you're having trouble fixing the issues, please let me know

@TheGittyPerson
Copy link
Copy Markdown
Owner

@kavyangaHA I will be closing this PR soon. Please leave a comment tagging me if you're able to work on this task again, or if you're busy and don't want to work on this task anymore anyway 👍

@TheGittyPerson
Copy link
Copy Markdown
Owner

Please open a new PR when you're back, addressing my review comments. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or enhancement task This is a task issue / PR completing a task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TASK: Add smile method

2 participants