-
Notifications
You must be signed in to change notification settings - Fork 5
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
Marcus/Replace all instances of person with task #107
Marcus/Replace all instances of person with task #107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good to me. Just have a few comments below.
Need to change some other things to tasks in the future as well.
docs/tutorials/AddRemark.md
Outdated
@@ -308,9 +308,9 @@ Just add [this one line of code!](https://github.com/se-edu/addressbook-level3/c | |||
**`PersonCard.java`:** | |||
|
|||
``` java | |||
public PersonCard(Person person, int displayedIndex) { | |||
public PersonCard(Person task, int displayedIndex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still not changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are in addremark.md not actual code, not sure if it needs to be changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok undid changes in these documents, please review again
docs/tutorials/AddRemark.md
Outdated
Person personToEdit = lastShownList.get(index.getZeroBased()); | ||
Person editedPerson = new Person(personToEdit.getName(), personToEdit.getPhone(), personToEdit.getEmail(), | ||
personToEdit.getAddress(), remark, personToEdit.getTags()); | ||
Person taskToEdit = lastShownList.get(index.getZeroBased()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class here is still not changed to Task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same reply as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok undid changes in these documents, please review again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now
Fix #99