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.

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

DG: Add implementation for "contactTaskTag", add more use stories, MSS, NFRs, Glossary, and Manual Testing #239

Merged

Conversation

lerxcl
Copy link
Collaborator

@lerxcl lerxcl commented Nov 7, 2020

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #239 (e626147) into master (193cb2b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #239   +/-   ##
=========================================
  Coverage     67.18%   67.18%           
  Complexity      887      887           
=========================================
  Files           130      130           
  Lines          3020     3020           
  Branches        469      469           
=========================================
  Hits           2029     2029           
  Misses          783      783           
  Partials        208      208           
Impacted Files Coverage Δ Complexity Δ
...main/java/seedu/address/model/task/Recurrence.java 51.85% <ø> (ø) 10.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 193cb2b...e626147. Read the comment docs.

Copy link
Collaborator

@urieltan urieltan 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 to merge!

@urieltan urieltan merged commit 339f8c2 into AY2021S1-CS2103T-F12-4:master Nov 7, 2020
Copy link
Collaborator

@luciatirta luciatirta 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 overall, just need small changes.

4. A user with above average typing speed for regular English text (i.e. not code, not system admin commands) should be able to accomplish most of the tasks faster using commands than using the mouse.
5. Commands should be intuitive so that users can quickly remember the commands.
6. Should work without an Internet connection.
7. Should not require more than 1 GB of storage space.
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps consider smaller storage space required since our jar file is only 13MB

Expected: First contact is deleted from the list. Details of the deleted contact shown in the status message. Timestamp in the status bar is updated.

1. Test case: `delete 0`<br>
1. Test case: `delete contact 0`<br>
Expected: No person is deleted. Error details shown in the status message. Status bar remains the same.

1. Other incorrect delete commands to try: `delete`, `delete x`, `...` (where x is larger than the list size)<br>
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the case of "delete x", it does not matter whether x is larger than the list size. Perhaps change the "delete x" to "delete contact x" or remove the phrase in bracket.

!include style.puml
skinparam arrowThickness 1.1
skinparam arrowColor MODEL_COLOR
skinparam classBackgroundColor MODEL_COLOR
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since command is part of the logic component, perhaps change the color to LOGIC_COLOR

!include style.puml
skinparam arrowThickness 1.1
skinparam arrowColor MODEL_COLOR
skinparam classBackgroundColor MODEL_COLOR
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since parser is part of the logic component, perhaps change the color to LOGIC_COLOR

| `*` | user with many contacts in the Lifebook | sort persons by name | locate a person easily |
| `*` | student with weekly lectures and tutorials | add recurring tasks | save time by not adding the same task every week, which is time-consuming|
| `*` | student | have a common tag for my contact and task | easily search for the associated contacts with a task |
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like the phrase "easily search for the associated contacts with a task" is too technical and not user-oriented. Perhaps can change to something like "easily find a person I am working with in a project"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants