-
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
Add Remark command #129
Add Remark command #129
Conversation
- Add `remark` field to Person - Add `remark` command to add / edit remarks - Update UI to reflect remarks (to be changed) To do: - Change tests to cater additional field
Codecov ReportAttention:
... and 2 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
* Changes the remark of an existing person in the address book. | ||
*/ | ||
public class RemarkCommand extends Command { | ||
public static final String COMMAND_WORD = "remark"; |
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.
Might make more sense to put this in the MESSAGES class
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.
Putting COMMAND_WORD
in the class itself seems to be the pattern for other commands too. Can you clarify why it should be shifted to the MESSAGES
class?
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.
I think Rohan was confused by the use of COMMAND_WORD
being able to be publicly accessed by other classes, I don't see an issue here as this is the command that we will use for the feature.
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!
public static final String MESSAGE_ADD_REMARK_SUCCESS = "Added remark to Person: %1$s"; | ||
public static final String MESSAGE_DELETE_REMARK_SUCCESS = "Removed remark from Person: %1$s"; |
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.
Nice way to show that the command succeeded
public final String value; | ||
|
||
/** |
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.
Seems like there is no regex for this field, which makes sense since is just a remark of a person
Closes #86
remark
command to add remarks to Person