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.

Sign up for GitHub

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

Jump to bottom

Add Reminder Class #73

Merged
merged 27 commits into from Oct 13, 2019
Merged

Add Reminder Class #73

merged 27 commits into from Oct 13, 2019

Conversation

TSAI-HSIAO-HAN
Copy link

@TSAI-HSIAO-HAN TSAI-HSIAO-HAN commented Oct 8, 2019 •
edited

Still need to add more method for Reminder and Note.

  • add ReminderTest

Na-Nazhou
Na-Nazhou reviewed Oct 9, 2019
View reviewed changes
Copy link

@Na-Nazhou Na-Nazhou left a comment

Choose a reason for hiding this comment

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

I am not sure about the java Date class, do they include time as well or just date?

src/main/java/seedu/mark/model/reminder/Reminder.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Reminder.java Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Reminder.java Outdated Show resolved Hide resolved
@TSAI-HSIAO-HAN @Na-Nazhou
Apply suggestions from code review
43b4982 
Co-Authored-By: Na Nazhou <42093424+Na-Nazhou@users.noreply.github.com>
@openorclose
Copy link

[Help Request] I tried to use the similar test as bookmark, but the coverage of reminder is quite low. #

To get more code coverage, you need to make sure more methods of Reminder are called.

Na-Nazhou
Na-Nazhou previously requested changes Oct 12, 2019
View reviewed changes
src/main/java/seedu/mark/model/reminder/Note.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Note.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Note.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Note.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Reminder.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Reminder.java Outdated Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Reminder.java Show resolved Hide resolved
src/main/java/seedu/mark/model/reminder/Reminder.java Outdated Show resolved Hide resolved
src/test/java/seedu/mark/testutil/ReminderBuilder.java Outdated Show resolved Hide resolved
@TSAI-HSIAO-HAN @Na-Nazhou
Apply suggestions from code review
6ae696a 
Co-Authored-By: Na Nazhou <42093424+Na-Nazhou@users.noreply.github.com>
@TSAI-HSIAO-HAN TSAI-HSIAO-HAN changed the title WIP: Add Reminder Class Add Reminder Class Oct 13, 2019
kxyoke
kxyoke reviewed Oct 13, 2019
View reviewed changes
Copy link

@kxyoke kxyoke left a comment

Choose a reason for hiding this comment

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

Looks good for now, we can make further changes as you add the command and fit them together

src/main/java/seedu/mark/model/reminder/Reminder.java Show resolved Hide resolved
@TSAI-HSIAO-HAN @kxyoke
Update src/main/java/seedu/mark/model/reminder/Reminder.java
675499c 
Co-Authored-By: Kappayolk <45960468+kxyoke@users.noreply.github.com>
v1.2 automation moved this from Review in progress to Reviewer approved Oct 13, 2019
@TSAI-HSIAO-HAN TSAI-HSIAO-HAN merged commit 6ba8738 into AY1920S1-CS2103T-T13-4:master Oct 13, 2019
v1.2 automation moved this from Reviewer approved to Done Oct 13, 2019
@Na-Nazhou Na-Nazhou added this to the v1.2 milestone Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Na-Nazhou Na-Nazhou Na-Nazhou left review comments

@dorcastan dorcastan dorcastan left review comments

@kxyoke kxyoke kxyoke approved these changes

Assignees
No one assigned
Labels
PR: merged
Projects
No open projects
v1.2
  
Done
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@TSAI-HSIAO-HAN @dorcastan @openorclose @Na-Nazhou @kxyoke