[F10-1] OASIS #86
Open
[F10-1] OASIS #86
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
-
Overall:
Good job on meeting all requirements for the v1.1 team component. However, some areas need more work. Hope you can tackle those areas in upcoming features. -
ReadMe adoc
- Remove the top SE EDU nav bar and link to LOs
- ContactUs adoc
- Update contact details
- UserGuide adoc
- Give examples for commands. Otherwise it can be difficult to follow during testing
docs/DeveloperGuide.adoc
Outdated
| |`* * *` |new user |see usage instructions |refer to instructions when I forget how to use the App | ||
| |`* * *` |User |See usage instructions |Refer to instructions when I forget how to use the application | ||
|
|
||
| |`* * *` |User |Log in |Access the features of the system |
There was a problem hiding this comment.
Please do have a work-around ready during the final testing phase. Your peers wouldn't want to go through the hassles of signing up when they are testing. May be provide dummy login credentials.
docs/UserGuide.adoc
Outdated
| * Selects the person and loads the Google search page the person at the specified `INDEX`. | ||
| * The index refers to the index number shown in the displayed person list. | ||
| * The index *must be a positive integer* `1, 2, 3, ...` | ||
| * Edits the person at the specified `INDEX`. The index refers to the index number shown in the displayed person list. The index *must be a positive integer* 1, 2, 3, ... |
There was a problem hiding this comment.
Wrong description for sort?
docs/UserGuide.adoc
Outdated
| Selects the person identified by the index number used in the displayed person list. + | ||
| Format: `select INDEX` | ||
| To use this command, you must be logged in as a manager. | ||
| By default, this sorts by their profitability, but it can also sort by details like name by specifying it in the -s parameter. |
There was a problem hiding this comment.
Give an example for the sort by parameter case also
There was a problem hiding this comment.
Feedback for v1.2
Contact Us
- Update link to issue tracker
User Guide
- For the filter by department command, what does the employee refer to get the department name?
Developer Guide
- Remove/Update the Design section (Section 2) to match your product
- Follow the UML notations for activity diagrams discussed in lecture and tutorial
- Have uniformity in using either activity diagram (AD) or sequence diagram (SD) throughout the guide. For e.g. Project Management feature uses AD, while Login/Logout feature has SD. Plus follow the same style in the diagrams (Black&White or Colored)
- Comments in Login/Logout feature SD
- Leave Application feature SD
- May remove Appendix A
Overall
- Good job on adding/udating numerous UML diagrams. The are mostly OK; just some minor things to fix. 👍
- Good job on drawing sequence diagrams, but we discourage low level diagrams that span multiple components. Try to draw diagrams at multiple levels: one that shows component level interactions and others shown low level interactions within one components (e.g. Logic).
- Let's push the product towards a user-testable state by v1.3
bannified
pushed a commit
to bannified/addressbook-level4
that referenced
this pull request
Oct 28, 2018
[V1.3] UI Update & Refactor
There was a problem hiding this comment.
Mid v1.4 Review
- LoggedInUI is missing from the Read Me (first) page of your product website.
- Dev Guide: Need to change/update Fig 2-6 as they still refer to AB-4 components and events.
- The class diagram of Model and UI in "View deleted employee archive" feature is chipped. Please zoom it out.
- For Login sequence diagram please mention which component does the sequence diagram refer to
- Complete SD for Leave Application. Also note that CS2103 encourages multi-level sequence diagrams. Higher level interactions between components (e.g. Logic and Model) in a separate diagram and lower level interactions between sub-components (e.g. Parser and Command) within a component (Logic) in a separate diagram. The lower level diagram should not include interactions between components (e.g. Logic and Model).
Final update
…ments # Conflicts: # docs/DeveloperGuide.adoc # docs/UserGuide.adoc
Add tag for viewpermission
Added viewpermission into Contributions to DG
Update PPP length
Remove viewpermission
Shrink to 10
grammar fix
UI class diagram updated
Update PPP
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@elliottan @KokJianYu @RnardHa @jroberts91