[CS2103T-F09-3] iGrad #35

dargohzy · 2020-02-19T03:54:36Z

Team PR for CS2103T.
@nathanaelseen
@teriaiw
@waynewee
@yjskrs

zsoh97

Overall well kept developer guide :)

zsoh97 · 2020-03-27T01:24:45Z

docs/DeveloperGuide.adoc

 * can save `UserPref` objects in json format and read it back.
-* can save the Address Book data in json format and read it back.
+* can save the Course Book data in json format and read it back.



missing structure of services component

zsoh97 · 2020-03-27T01:29:09Z

docs/DeveloperGuide.adoc

 image::UndoRedoState0.png[]

-Step 2. The user executes `delete 5` command to delete the 5th person in the address book. The `delete` command calls `Model#commitAddressBook()`, causing the modified state of the address book after the `delete 5` command executes to be saved in the `addressBookStateList`, and the `currentStatePointer` is shifted to the newly inserted address book state.
+<<<<<<< HEAD Step 2. The user executes `delete 5` command to delete the 5th module in the course book.


did not merge correctly.

zsoh97 · 2020-03-27T01:29:42Z

docs/DeveloperGuide.adoc


-Step 2. The user executes `delete 5` command to delete the 5th person in the address book. The `delete` command calls `Model#commitAddressBook()`, causing the modified state of the address book after the `delete 5` command executes to be saved in the `addressBookStateList`, and the `currentStatePointer` is shifted to the newly inserted address book state.
+<<<<<<< HEAD Step 2. The user executes `delete 5` command to delete the 5th module in the course book.
+The `delete` command calls `Model#commitCourseBook()`, causing the modified state of the course book after the `delete 5` command executes to be saved in the `addressBookStateList`, and the `currentStatePointer` is shifted to the newly inserted course book state.


3.3 to 3.3.2: information was updated to be relevant to product but diagrams for this section was not updated to reflect the product.

zsoh97 · 2020-03-27T01:33:45Z

docs/DeveloperGuide.adoc

+This would allow `Course Book` to only require one `Tag` object per unique `Tag`, instead of each `Module` needing their own `Tag` object.
+An example of how such a model may look like is given below. +
 +
 image:BetterModelClassDiagram.png[]


compared to above model diagram, there are additional components of person, name, email, address, unique tag list and unique person list. which model diagram should the reader follow? should be reflected in earlier diagram.

I think that they simply haven't updated their diagram yet, since Person does not exist in the context of iGrad

zsoh97 · 2020-03-27T01:39:02Z

docs/DeveloperGuide.adoc

+
+The sequence diagram below provide further insight into its execution:
+
+image::AutoAddSequenceDiagram.png[]


if parseCommand parameter is string it should be in "". However, the param is stated as auto add with a space.

zsoh97 · 2020-03-27T01:41:34Z

docs/DeveloperGuide.adoc

+In such situations, the user would be prompted to manually fill in the necessary details for a module. The compulsory
+and optional fields are shown in the class diagram below:
+
+image::ModuleClassDiagram.png[]


figure is not labelled.

zsoh97 · 2020-03-27T01:44:37Z

docs/DeveloperGuide.adoc

-.  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.
+. Should work on any <<mainstream-os,mainstream OS>> as long as it has Java `11` or above installed.
+. Should be able to hold up to 100 modules without a noticeable sluggishness in performance (i.e. should take less than 1 second to load)
+. A user with above 70 wpm 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.


wpm not defined.

NNpanpan

Diagrams can have a bit larger fonts so that readers don't have to zoom in.
Other things look good overall.

NNpanpan · 2020-03-27T02:28:44Z

docs/DeveloperGuide.adoc

+In such situations, the user would be prompted to manually fill in the necessary details for a module. The compulsory
+and optional fields are shown in the class diagram below:
+
+image::ModuleClassDiagram.png[]


Notations on Diagram are not the standard notations.

NNpanpan · 2020-03-27T02:29:44Z

docs/DeveloperGuide.adoc

+Here is how the requirement class updates when a requirement is added:
+
+.Sequence Diagram when adding a requirement.
+image::RequirementAddSequenceDiagram.png[]


Doesn't need new keyword when calling constructors.

NNpanpan · 2020-03-27T02:31:11Z

docs/DeveloperGuide.adoc

+
+The sequence diagram below provide further insight into its execution:
+
+image::AutoAddSequenceDiagram.png[]


Diagram lacks a label.

NNpanpan · 2020-03-27T02:42:07Z

docs/DeveloperGuide.adoc

 [TIP]
 The `.puml` files used to create diagrams in this document can be found in the link:{repoURL}/docs/diagrams/[diagrams] folder.
 Refer to the <<UsingPlantUml#, Using PlantUML guide>> to learn how to create and edit diagrams.


Parts like this should be left out.

dinhnhobao

Looks good to me overall!

dinhnhobao · 2020-03-27T02:31:18Z

docs/DeveloperGuide.adoc

 === Model component

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


Should the class diagram avoid long and curly arrows, such as
ModelManager to Module and Requirement?

dinhnhobao · 2020-03-27T02:33:45Z

docs/DeveloperGuide.adoc

+This would allow `Course Book` to only require one `Tag` object per unique `Tag`, instead of each `Module` needing their own `Tag` object.
+An example of how such a model may look like is given below. +
 +
 image:BetterModelClassDiagram.png[]


I think that they simply haven't updated their diagram yet, since Person does not exist in the context of iGrad

dinhnhobao · 2020-03-27T02:40:30Z

docs/DeveloperGuide.adoc

+
+The following activity diagram illustrates this:
+
+image::AutoAddActivityDiagram.png[]


Is the activity diagram complete?
What if "the previous academic year" request still does not return a valid response? Do we make another API call for previous previous academic year?

dinhnhobao · 2020-03-27T02:41:48Z

docs/DeveloperGuide.adoc

+Here is how the requirement class updates when a requirement is added:
+
+.Sequence Diagram when adding a requirement.
+image::RequirementAddSequenceDiagram.png[]


same comment as @zsoh97 for AutoAddSequenceDiagram.png

dinhnhobao · 2020-03-27T02:42:36Z

docs/DeveloperGuide.adoc

+
+Step 1: The RequirementAddCommandParser is called to parse the RequirementAddCommand with the `n/` and the `u/` prefixes into a new requirement.
+
+Step 2: The RequirementAddCommand is executed to add the new requirement to the model. In this step, the following check is performed:


Should the class names be under Markdown syntax ?

… into requirements

…rs-requirements

added moduleaddautocommand added modulefiltercommand added undocommand added exportcommand help

Updating Local Repo

…-master

Updating Local Repo

Fix travis

Update documents

Final pr

zsoh97 reviewed Mar 27, 2020

View reviewed changes

NNpanpan reviewed Mar 27, 2020

View reviewed changes

dinhnhobao reviewed Mar 27, 2020

View reviewed changes

Nathanael Seen and others added 27 commits April 9, 2020 22:03

Merge branch 'dargohzy-master'

3fcf677

Comment out module auto add command test

920c3f5

Merge branch 'master' into update-tests-for-course-info

109e8a7

Add ppp

072d371

Merge branch 'master' of https://github.com/AY1920S2-CS2103T-F09-3/main…

fe5971a

… into requirements

Clean up unnecessary tests in RequirementAssignCommandParserTest.java

65aa73e

Merge branch 'requirements' of git://github.com/yjskrs/main into yjsk…

49c03c9

…rs-requirements

Merge branch 'yjskrs-requirements'

fea36cf

Merge branch 'master' into update-tests-for-course-info

6f46d3c

added module

8169745

added moduleaddautocommand added modulefiltercommand added undocommand added exportcommand help

fix checkstyle errors

3ee6aa4

Merge branch 'master' into dev-guide-final-pls

9756af4

added ppp

e9b3996

Merge branch 'master' into wayne-ppp

1005b5d

Merge pull request #29 from AY1920S2-CS2103T-F09-3/master

7a42c8a

Updating Local Repo

Added module parser tests + author tags

ca0d537

Merge remote-tracking branch 'origin/master'

2fa0ae0

Update RequirementAssignCommandParserTest.java tests

72e19c6

Merge branch 'dev-guide-final-pls'

1ff8a3c

Fix merge conflicts and compilation errors

7d5b710

Merge branch 'wayne-ppp'

66bf1f2

Merge branch 'master' of git://github.com/dargohzy/main into dargohzy…

2421f44

…-master

Fix checkstyle

cbb9c7b

Merge branch 'dargohzy-master'

86bdf96

Merge branch 'master' into update-tests-for-course-info

8e85734

Add equals method for RequirementAssignCommand

f8ca79d

Further remodularise parsing mechanism in RequirementAssign cmd

8a8f31c

teriaiw and others added 30 commits April 13, 2020 20:11

Update PPP and DG

b33a4a7

Update DG Requirement Add

72f4116

Minor fixes in diagram

e1f60fa

add quickstart

6b2b795

Complete my parts for DG

f60ddc0

Add UG

8282815

Merge branch 'update-docs-teri'

afd1f21

Fix merge conflicts

eda0251

Merge branch 'master' into quickstart

691d277

Merge branch 'quickstart'

30726ae

Merge branch 'master' into update-dg-nat

004a8f6

Fix DG minor issues

99dc35a

Merge branch 'update-dg-nat'

a320e85

Delete NatDG.adoc

5608472

Complete UG

4eca46d

Merge Conflict

17c6fac

Shorten PPP

a92e610

Merge branch 'shorten-nat-ppp'

14dbade

Update PPP

9c22fdb

Update UG

03d8284

Update PPP

eb39470

Merge pull request #34 from AY1920S2-CS2103T-F09-3/master

cdb4df6

Updating Local Repo

fix travis

219050d

fix travis

895974e

Update documents

c58d78e

Merge pull request #333 from dargohzy/master

d3ecf53

Fix travis

Merge pull request #334 from yjskrs/documents

8c6c1e0

Update documents

Remove redundant section in DG

b815c98

Last commit

99c0d93

Merge pull request #335 from AY1920S2-CS2103T-F09-3/final-pr

b0874e7

Final pr


		The sequence diagram below provide further insight into its execution:

		image::AutoAddSequenceDiagram.png[]


		The following activity diagram illustrates this:

		image::AutoAddActivityDiagram.png[]


		Step 1: The RequirementAddCommandParser is called to parse the RequirementAddCommand with the `n/` and the `u/` prefixes into a new requirement.

		Step 2: The RequirementAddCommand is executed to add the new requirement to the model. In this step, the following check is performed:

[CS2103T-F09-3] iGrad #35

Are you sure you want to change the base?

[CS2103T-F09-3] iGrad #35

Uh oh!

Conversation

dargohzy commented Feb 19, 2020

Uh oh!

zsoh97 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NNpanpan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dinhnhobao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants