-
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 Unit Number #59
Add Unit Number #59
Conversation
Update CLISyntax.java: Add prefix for unit number Add UnitNumber.java: Add functionality to add unit number to add command Add UnitNumberTest.java: Add test cases to test UnitNumber.java
Update UnitNumberTest.java: Remove two test cases
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #59 +/- ##
============================================
+ Coverage 75.79% 76.20% +0.40%
- Complexity 431 450 +19
============================================
Files 72 74 +2
Lines 1376 1416 +40
Branches 131 135 +4
============================================
+ Hits 1043 1079 +36
- Misses 303 307 +4
Partials 30 30 ☔ View full report in Codecov by Sentry. |
Update UnitNumber.java: Change the requirement such that unit number must be at least 1 digit, and at most 3 digits (but cannot be 0) Update UnitNumberTest.java: Change the test cases according to the new requirements
Update UnitNumber.java: Add another regex to test for the following input '0', '00', '000', which should be invalid since a unit number cannot be '0's Update UnitNumberTest.java: Update the test cases according to the new requirements
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 me 👍
Update ParserUtil.java: Add parsing of unit numbers Update ParserUtilTest.java: Add test cases for parsing of unit numbers
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
@@ -11,6 +11,7 @@ public class CliSyntax { | |||
public static final Prefix PREFIX_EMAIL = new Prefix("e/"); | |||
public static final Prefix PREFIX_ADDRESS = new Prefix("a/"); | |||
public static final Prefix PREFIX_TAG = new Prefix("t/"); | |||
public static final Prefix PREFIX_UNITNUMBER = new Prefix("unitNo/"); |
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.
Should this be formatted like 'u/' to maintain consistency with the rest of the syntax?
Update CLISyntax.java: Add prefix for unit number
Add UnitNumber.java: Add functionality to add unit number to add command
Add UnitNumberTest.java: Add test cases to test UnitNumber.java