[W5.7][T12-3]Thaddeus Lim #437
Conversation
Updated User Guide on Count Command
Added new command 'count' under commands
Added new help command for 'count'
Added new Field 'sumPersons' to keep track of total person count
Imported CountCommand
Updated tests to test for the new Count method.
|
Hi, from the travis, the follow is not a statement which seems to be the error, you may want to fix it and check if it can pass the travis test [assertTrue(defaultAddressBook.getSumPersons()) == 0;] |
Corrected a syntax error at line 143
|
Seems like the the test case has failed? |
Small changes in attempt to pass Travis test
Minor changes to code format
Edited test cases to pass
There was a problem hiding this comment.
Good effort on this PR @thaddeuslzy. In particular, good job on,
- updating user docs
- updating I/O tests
Suggestions to improve further:
- update dev docs
- include more jUnit and I/O tests to test the new functionality
- limit the PR to one enhancement and avoid including changes not related to that enhancement or unmerged code from other PRs
- pay closer attention to the coding standard
Please close the PR after viewing comments.
| @@ -156,6 +156,11 @@ Views all details of the 1st person in the results of the `find` command. | |||
| Clears all entries from the address book. + | |||
| Format: `clear` | |||
|
|
|||
| == Counting total number of persons in the address book : `count` | |||
| Counts total number of persons in the address book. | |||
| Format: `count` | |||
There was a problem hiding this comment.
Good job updating the user doc
| Counts total number of persons in the address book. | ||
| Format: `count` | ||
|
|
||
|
|
There was a problem hiding this comment.
Stray line break? Try to avoid these spaces by checking diff while submitting a PR.
| */ | ||
| public class CountCommand extends Command { | ||
|
|
||
| public static final String COMMAND_WORD = "count"; |
There was a problem hiding this comment.
Try to avoid public class member variables
| @@ -36,11 +35,19 @@ public void elementsAreUnique() throws Exception { | |||
| assertNotUnique(null, "a", "b", null); | |||
| } | |||
|
|
|||
| @Test | |||
| public void isAnyNull() throws Exception { | |||
There was a problem hiding this comment.
This commit shouldn't be part of this PR. In general, a PR should not contain unmerged code from other PRs. Try to limit each PR to one enhancement, which means the PR should be based on a branch that starts off from the master branch that is in sync with the upstream master branch
|
Hi Ms Pavarthy,
Can I check with you with regard to the participation marks for this LO? Because I did not get the participation marks when I checked the grade book. I replied to the 2103-bot, but I never got a reply. Thank you!
Best Regards,
Thaddeus Lim
On 16 Sep 2018, at 10:53 PM, Parvathy Sudhir Pillai <[email protected]<mailto:[email protected]>> wrote:
@parvathy-sudhir-pillai approved this pull request.
Good effort on this PR @thaddeuslzy<https://github.com/thaddeuslzy>. In particular, good job on,
* updating user docs
* updating I/O tests
Suggestions to improve further:
* update dev docs
* include more jUnit and I/O tests to test the new functionality
* limit the PR to one enhancement and avoid including changes not related to that enhancement or unmerged code from other PRs
* pay closer attention to the coding standard
Please close the PR after viewing comments.
________________________________
In docs/UserGuide.adoc<#437 (comment)>:
@@ -156,6 +156,11 @@ Views all details of the 1st person in the results of the `find` command.
Clears all entries from the address book. +
Format: `clear`
+== Counting total number of persons in the address book : `count`
+Counts total number of persons in the address book.
+Format: `count`
Good job updating the user doc
________________________________
In docs/UserGuide.adoc<#437 (comment)>:
@@ -156,6 +156,11 @@ Views all details of the 1st person in the results of the `find` command.
Clears all entries from the address book. +
Format: `clear`
+== Counting total number of persons in the address book : `count`
+Counts total number of persons in the address book.
+Format: `count`
+
+
Stray line break? Try to avoid these spaces by checking diff while submitting a PR.
________________________________
In src/seedu/addressbook/commands/CountCommand.java<#437 (comment)>:
@@ -0,0 +1,21 @@
+package seedu.addressbook.commands;
+
+/**
+ * Shows total number of persons in address book.
+ */
+public class CountCommand extends Command {
+
+ public static final String COMMAND_WORD = "count";
Try to avoid public class member variables
________________________________
In test/java/seedu/addressbook/common/UtilsTest.java<#437 (comment)>:
@@ -36,11 +35,19 @@ public void elementsAreUnique() throws Exception {
assertNotUnique(null, "a", "b", null);
}
+ @test
+ public void isAnyNull() throws Exception {
This commit shouldn't be part of this PR. In general, a PR should not contain unmerged code from other PRs. Try to limit each PR to one enhancement, which means the PR should be based on a branch that starts off from the master branch that is in sync with the upstream master branch
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#437 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AmdyUamC_ZzH_Ynhq1dw0taNN4HP_Vxcks5ubmXggaJpZM4WmaK5>.
|
Hi Thaddeus, The participation marks for the LO are assigned by the PR bot, which assigns marks for all those PRs created by you till the midnight before the tutorial. This means, your PR with the commits should have been created before Wed, Sept 12 23.59 PM. The bot may not have assigned you marks since your commits are majorly on Sept 13. Also, the PR review (not the participation marks assignment) is done by me at a much later date (Sun, Sept 16). The PR bot awarding participation marks to you and the one assigning me as the reviewer are different and don't communicate. |
Added new Count Command to the AddressBook