Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[W5][T08-1]Jeremy Wong #54

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

articstranger
Copy link

No description provided.

@swalahlah
Copy link

A simple but important tool of an element that we, as software engineers, often miss out. A simple greeting can make its user feels welcome.

Copy link

@RubaP RubaP left a comment

Choose a reason for hiding this comment

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

Good attempt. But try to do more meaningful enhancements to the functionalities in future, rather than a simple greeting so that it will help you more to learn the SE practices.

@@ -156,6 +156,11 @@ Views all details of the 1st person in the results of the `find` command.
Clears all entries from the address book. +
Format: `clear`

== Saying hi : `hello`

Copy link

Choose a reason for hiding this comment

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

Good that you have updated the UserGuide

@@ -75,6 +65,12 @@ public void parse_clearCommand_parsedCorrectly() {
parseAndAssertCommandType(input, ClearCommand.class);
}

@Test
public void parse_helloCommand_parsedCorrectly() {
Copy link

Choose a reason for hiding this comment

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

Good job for adding both I/O and JUnit tests

@RubaP RubaP added the Reviewed label Feb 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants