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

[F10-3] Heart² #77

Open
wants to merge 904 commits into
base: master
Choose a base branch
from

Conversation

liaujianjie
Copy link

45598280-f0abd600-ba0b-11e8-8202-2c607cf221ea


@eehooi @wailunlim @NightYeti @dongsiji @liaujianjie

Copy link

@dalessr dalessr left a comment

Choose a reason for hiding this comment

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

Nice work! Some suggestions below.

Also, please update user guide (i.e. features for each version) and developer guide (i.e. user story, use cases, NFR...) accordingly.

README.adoc Outdated
@@ -1,7 +1,7 @@
= Address Book (Level 4)
Copy link

Choose a reason for hiding this comment

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

Can change it to the name of your app. Remember to change all Address Book Level 4 in your project

README.adoc Outdated
@@ -1,7 +1,7 @@
= Address Book (Level 4)
ifdef::env-github,env-browser[:relfileprefix: docs/]

https://travis-ci.org/se-edu/addressbook-level4[image:https://travis-ci.org/se-edu/addressbook-level4.svg?branch=master[Build Status]]
https://travis-ci.org/CS2103-AY1819S1-F10-3/main[image:https://travis-ci.org/CS2103-AY1819S1-F10-3/main.svg?branch=master[Build Status]]
https://ci.appveyor.com/project/damithc/addressbook-level4[image:https://ci.appveyor.com/api/projects/status/3boko2x2vr5cc3w2?svg=true[Build status]]
https://coveralls.io/github/se-edu/addressbook-level4?branch=master[image:https://coveralls.io/repos/github/se-edu/addressbook-level4/badge.svg?branch=master[Coverage Status]]
Copy link

Choose a reason for hiding this comment

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

Can change all these links to your own project's build. (For Appveyor can use team lead's project link)

@@ -4,53 +4,48 @@
:imagesDir: images
:stylesDir: stylesheets

AddressBook - Level 4 was developed by the https://se-edu.github.io/docs/Team.html[se-edu] team. +
_{The dummy content given below serves as a placeholder to be used by future forks of the project.}_ +
Heart² was developed by the F10-3 team. +
Copy link

Choose a reason for hiding this comment

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

Is it better to specify as NUS School of Computing AY1819...?

{empty}[http://www.comp.nus.edu.sg/~damithch[homepage]] [https://github.com/damithc[github]] [<<johndoe#, portfolio>>]
=== Dong SiJi
image::dongsiji.jpg[width="150", align="left"]
{empty}[https://github.com/dongsiji[github]] [<<johndoe#, portfolio>>]
Copy link

Choose a reason for hiding this comment

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

Why here johndoe# still appears?


Role: Project Advisor
Role: In charge of ...
Copy link

Choose a reason for hiding this comment

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

Can decide the role now. Feel free to modify if there are changes during the process.

Copy link

@dalessr dalessr 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 👍 @eehooi @wailunlim @NightYeti @dongsiji @liaujianjie

Some comments:

  • AboutUs page: assign roles and components to specific team members (i.e. role: team lead/developer, component: storage/model/logic...)
  • User Guide: add description and examples for all the features due to consistency. Can omit examples of trivial features (i.e. exit). Similar to the other features, features for v2.0 should also have description, format & examples.
  • Developer Guide: some use cases are still incorrect. Remember that the steps in MSS reflect the interaction between user and system, which indicates that 2 system actions cannot appear consecutively. Can remove Appendix G if not needed.

Copy link

@dalessr dalessr left a comment

Choose a reason for hiding this comment

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

@eehooi @wailunlim @NightYeti @dongsiji @liaujianjie Some comments:

Contact Us:

  • Link to the issue tracker should be the link of your team repo's issue tracker instead of se-edu

User Guide:

  • For consistency, add examples for every non-trivial commands (i.e. login)
  • Add description, format and example for 3.13 also, even though you will not be implementing it.

Developer Guide:

  • Update/add diagrams where applicable. Some class/sequence diagrams do not follow your current product any more. i.e.:
    image1

image2

  • Add labels for all the newly added diagrams

Bellaaarh pushed a commit to Bellaaarh/addressbook-level4 that referenced this pull request Oct 29, 2018
eehooi and others added 30 commits November 12, 2018 19:33
Remove delete manual testing and fix minor PPP details
Disallow sensitive commands from storing parameters to history
Correct spelling and some grammar
Add manual testing for add, delete, update,view
Update DG to tell user how to change default root account credentials
Update undoredo sequence diagram
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants