-
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
Fix bugs #189, #206, #214, #219 + PPP #234
Conversation
… acceptable range of inputs for AppointmentDateTime.
# Conflicts: # src/main/java/seedu/address/model/appointment/AppointmentDateTime.java
Codecov Report
@@ Coverage Diff @@
## master #234 +/- ##
============================================
- Coverage 62.33% 62.29% -0.05%
Complexity 768 768
============================================
Files 120 120
Lines 2979 2981 +2
Branches 347 348 +1
============================================
Hits 1857 1857
- Misses 1004 1005 +1
- Partials 118 119 +1
Continue to review full report at Codecov.
|
Do not merge yet. I will need to modify mine after Moon's. |
# Conflicts: # src/main/java/seedu/address/logic/parser/ParserUtil.java # src/main/java/seedu/address/model/appointment/AppointmentDateTime.java # src/test/java/seedu/address/logic/parser/ParserUtilTest.java
I realised when I merged Moon's PR to work on the modifications, the format is still following the old one so I updated the format here. |
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.
Hey Michaelia! The pr looks great. There's just one more thing to note about the AppointmentDateTime. Currently, AppointmentDateTime also accepts years beyond 19XX and 2XXX. Also, technically 30 or 31 might not be a valid input for day, depending on the month. So I was thinking you could maybe revert the changes from lines 17 to 21.
I forgot about it, lemme do amendments to it. |
Fix #189, #206, #214 , #219