-
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
Branch feature countdown #112
Branch feature countdown #112
Conversation
Add exceptions classes to support events list.
Add Event methods in Model, ModelManager, ReadOnlyTr4cker, Tr4cker, ReadOnlyUserPrefs, UserPrefs.
More crucial reason: May need to merge from upstream because I did not do so before starting this branch.
...but it doesnt show the events.
Codecov Report
@@ Coverage Diff @@
## master #112 +/- ##
============================================
- Coverage 63.57% 61.74% -1.84%
- Complexity 619 660 +41
============================================
Files 103 112 +9
Lines 2158 2363 +205
Branches 249 274 +25
============================================
+ Hits 1372 1459 +87
- Misses 706 802 +96
- Partials 80 102 +22
Continue to review full report at Codecov.
|
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.
Some minor changes and it LGTM.
@@ -46,6 +47,8 @@ | |||
/** Returns an unmodifiable view of the filtered list of tasks for PlannerDay. */ | |||
ObservableList<Task> getPlannerFilteredTaskList(); | |||
|
|||
/** Returns an unmodifiable view of the filtered list of events. */ | |||
ObservableList<Event> getFilteredEventList(); |
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.
Maybe can add a new line here for readability.
@@ -88,6 +89,9 @@ public ReadOnlyTr4cker getTr4cker() { | |||
} | |||
|
|||
@Override | |||
public ObservableList<Event> getFilteredEventList() { | |||
return model.getFilteredEventsList(); | |||
} |
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.
Here too! :')
ModelManager.java
too.
events.remove(key); | ||
} | ||
|
||
public Event firstEvent() { |
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.
Missing Javadocs for public methods.
public ObservableList<Event> getEventList() { | ||
events.sortEventsByDate(); | ||
return events.asUnmodifiableObservableList(); | ||
} |
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.
bump
public class EventName { | ||
|
||
public static final String MESSAGE_CONSTRAINTS = | ||
"Names should only contain alphanumeric characters and spaces, and it should not be blank"; |
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.
Maybe can specify "Event names should only..."?
} | ||
} | ||
|
||
public void setEvents(seedu.tr4cker.model.countdown.UniqueEventList replacement) { |
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.
Missing Javadocs.
And also the parameter in method signature should just be UniqueEventList
and then import seedu.tr4cker.model...
!
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.
Looks good to merge!
Im very sorrry
im still not done.
sorry.