Skip to content

Conversation

@BryanYap972
Copy link

NicholasCF added a commit to NicholasCF/main that referenced this pull request Mar 5, 2020
* update AboutUs

* update AboutUs

* update ContactUs

* update readme

* Fix checkstyle violations

Co-authored-by: Nicholas Cristian Fernando <[email protected]>
Copy link

@heidicrq heidicrq left a comment

Choose a reason for hiding this comment

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

Great job in general.

Comment on lines 319 to 322
|`* *` |student |add contact information on CS2103T professors |can contact them

|`* * *` |student |create meeting events | can keep track of my schedule

Copy link

Choose a reason for hiding this comment

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

User stories are not sorted by priority.

Comment on lines +323 to +326
|`* * *` |student |create study session events | can keep track of my schedule

|`* * *` |student |create consultation events | can keep track of my schedule

Copy link

Choose a reason for hiding this comment

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

Same benefit. Can add more details or be more specific about the benefits.

Comment on lines 508 to 509
. The application should respond to every command within one second.
. Technical requirements: The application should work on both 32-bit and 64-bit environments.
Copy link

Choose a reason for hiding this comment

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

Not app specific.

Copy link

@zixinn zixinn left a comment

Choose a reason for hiding this comment

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

Great job!


|`* * *` |student |delete classmate or teammate’s contact |delete if not necessary anymore

|`* * *` |student |categorise contacts into teammate or classmate |
Copy link

Choose a reason for hiding this comment

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

Missing benefit for user story

_{More to be added}_

[appendix]
== Non Functional Requirements
Copy link

Choose a reason for hiding this comment

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

NFR not app-specific

_{More to be added}_

[appendix]
== Glossary
Copy link

Choose a reason for hiding this comment

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

Important terms missing, e.g. CLI, GUI

Copy link

@alushingg alushingg left a comment

Choose a reason for hiding this comment

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

Good job


|`* * *` |student |edit profile picture to added contact |edit the picture if changes are necessary

|`* * *` |student |delete profile picture to added contact|

Choose a reason for hiding this comment

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

Missing benefit

_{More to be added}_

[appendix]
== Glossary

Choose a reason for hiding this comment

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

Maybe can add a glossary for MSS, Extensions

Copy link

@NNpanpan NNpanpan left a comment

Choose a reason for hiding this comment

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

Nice job, guys!


|`* * *` |student |delete profile picture to added contact|

|`* * *` |student |add category specific description |filter out a contact’s remark according to type of contact
Copy link

Choose a reason for hiding this comment

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

"Category specific" - Should clarify which objects' categories that are mentioned


|`* * *` |student |create lesson events | can keep track of my schedule

|`* * *` |student |note down the location of the meeting | know where to go
Copy link

Choose a reason for hiding this comment

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

What does "note down" mean in this system?

. Should be able to hold up to 1000 persons without a noticeable sluggishness in performance for typical usage.
. 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.
. The application should respond to every command within one second.
. Technical requirements: The application should work on both 32-bit and 64-bit environments.
Copy link

Choose a reason for hiding this comment

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

Given that almost all personal computers are running 64-bit OSes, is this a necessary constraint?

Comment on lines +321 to +327
|`* * *` |student |create meeting events | can keep track of my schedule

|`* * *` |student |create study session events | can keep track of my schedule

|`* * *` |student |create consultation events | can keep track of my schedule

|`* * *` |student |create lesson events | can keep track of my schedule

Choose a reason for hiding this comment

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

It would better if the "so that I..." part is more descriptive and detailed. This can help us to show that the user stories/features provided are very useful and specific for the target user.


|`* * *` |student |find events | can check if I have any specific events according to keyword(s)

|`* *` |student |add notes to events | can jot down additional details about the events

Choose a reason for hiding this comment

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

It would be useful to elaborate "jot down additional details" so as it's clearer as a benefit.

Copy link

@dinhnhobao dinhnhobao left a comment

Choose a reason for hiding this comment

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

Well done!

. In addition, the `CommandResult` object can also instruct the `Ui` to perform certain actions, such as displaying help to the user.

Given below is the Sequence Diagram for interactions within the `Logic` component for the `execute("delete 1")` API call.
Given below is the Sequence Diagram for interactions within the `Logic` component for the `execute("done t\2020-03-20 i\2")` API call.
Copy link

Choose a reason for hiding this comment

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

This sequence diagram looks too complicated. Perhaps you could abstract out some of the information and create another separate sequence diagram such that each diagram contains lesser and more relevant information.


5. `Model#markDone()` is called, and the `Schedule` object in `ModelManager` is updated.

The following sequence diagram shows how the mark event as done operation works:
Copy link

Choose a reason for hiding this comment

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

This diagram is repetitive and does not provide additional information.


*API* :
link:{repoURL}/src/main/java/seedu/address/logic/Logic.java[`Logic.java`]
link:{repoURL}/src/main/java/seedu/nova/logic/Logic.java[`Logic.java`]

Choose a reason for hiding this comment

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

The same diagram is duplicated above, in the higher-level description of the app's architecture. Perhaps a simpler and more abstract diagram could be used to describe the higher-level summary of Logic (and you can leave the more detailed diagram of Logic here)

Choose a reason for hiding this comment

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

but I think its good that you provided some notes for the XYZCommand (in the class diagram)

@@ -1,8 +1,9 @@
= AddressBook Level 3 - Developer Guide
= NOVA - Developer Guide
Copy link

Choose a reason for hiding this comment

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

The uml diagrams are not consistent - one is just white boxes but the other is white box highlighted with red lines. Maybe can make them all just non-highlight or make them all highlighted with different colours

=== Model component

.Structure of the Model Component
image::ModelClassDiagram.png[]
Copy link

Choose a reason for hiding this comment

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

What is the empty box between person and email and category?

BryanYap972 and others added 30 commits April 13, 2020 21:54
Replaced Ui.png with Windows version
DG: Fixed broken link in DG for Main.java and MainApp.java, Updated Ui.png Windows ver (with black border)
Fixed spaces in error message
* 'master' of https://github.com/AY1920S2-CS2103T-F10-3/main:
  updated dg
  updated dg
  Fixed progress calculation.
  fixed spaces in error message
  fixed spaces in error message
Edited Ui.png again, UG change: Changed team name (removed "Team" in front)
Edited UG: Command Summary to include Bryan's name
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
Change to 1.4.0 in MainApp.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.