[T12-2] PatientBook #61

pratyushghosh · 2018-09-19T18:21:37Z

No description provided.

eugenepeh

Just a brief look through. Great work so far.

Some things to note though,

usually user guide should match the current product, so if its not yet implemented, do mark it with a (future feature) or something. I see that you guys have done that but its not obvious enough :D
You can remove the top navigation bar too as it doesn't fit your product nicely.

Keep up the good work towards v1.1

eugenepeh · 2018-09-24T09:26:33Z

docs/AboutUs.adoc

-{empty}[http://www.comp.nus.edu.sg/~damithch[homepage]] [https://github.com/damithc[github]] [<<johndoe#, portfolio>>]
+=== Zhang Lizhi
+image::kumuwu.png[width="150", align="left"]
+{empty}[https://github.com/kumuwu[github]] [<<zhanglizhi#, portfolio>>]

 Role: Project Advisor


Remember to update the roles too

eugenepeh · 2018-09-24T09:28:40Z

README.adoc

 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]]
 https://www.codacy.com/app/damith/addressbook-level4?utm_source=github.com&utm_medium=referral&utm_content=se-edu/addressbook-level4&utm_campaign=Badge_Grade[image:https://api.codacy.com/project/badge/Grade/fc0b7775cf7f4fdeaf08776f3d8e364a[Codacy Badge]]
 https://gitter.im/se-edu/Lobby[image:https://badges.gitter.im/se-edu/Lobby.svg[Gitter chat]]
+image:https://img.shields.io/vscode-marketplace/r/ritwickdey.LiveServer.svg[Visual Studio Marketplace]
+image:https://img.shields.io/beerpay/hashdog/scrapfy-chrome-extension.svg[Beerpay]


are these badges relevant to your project?

eugenepeh · 2018-09-24T09:29:05Z

README.adoc

 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://ci.appveyor.com/project/damithc/addressbook-level4[image:https://ci.appveyor.com/api/projects/status/3boko2x2vr5cc3w2?svg=true[Build status]]


remember to update the link of original badges too.

eugenepeh · 2018-09-24T09:34:09Z

docs/DeveloperGuide.adoc

+.   Patient Management Mode: PatientDisplay
+.   Appointment Management Mode : CalendarDisplay
+
+*User Stories*


would be nice to follow the AB4 style to list it as a table with priorities.

eugenepeh · 2018-09-24T09:36:27Z

docs/DeveloperGuide.adoc

+.   As a hardworking doctor, I can print out the list of all my patients and hang it on my wall, sothat I am always motivated.
+.   As a forgetful doctor, I can append any note when I create an appointment, so that when I viewmy appointment I can quickly recall my purpose for the appointment.
+
+*Project Management*


Good work on coming up with project management plan, there's also inbuilt GitHub functionality though which you can consider instead of constantly updating the dev guide. e.g. https://github.com/nus-cs2103-AY1819S1/addressbook-level4/projects

eugenepeh · 2018-09-24T09:37:41Z

docs/DeveloperGuide.adoc

+* V1.2 Modify EventsCenter API calls, and create modify/create calls to accommodate new logging information for storage I/O.
+* V1.3 Format output for BrowserPanel. BrowserPanel will accept a person object (in the case of PatientDisplay) or an appointment object (in the case of Calendar display) and format the output appropriately either through Java or generating and displaying a temporary HTML document.
+
+Feature 4: Natural Language Processing for appointment dates


Can request to use some open source libraries instead.

damithc · 2018-10-02T07:48:09Z

Guys, the current version of your team repo is failing CI checks. Try to fix before v1.1

eugenepeh · 2018-10-08T09:13:04Z

@pratyushghosh @ZZG229 @lixinze777 @shuiherng @kumuwu
Do try to get your travis back up and running soon, its been failing for too long.

Look for ERROR in the travis log and rectify them.

eugenepeh

Good work, code quality wise seems to be good. Some things, however, need to be updated soon. E.g. Travis.

Other things:

Landing page
- url needs to be updated
Gh-pages
- Update/remove to match your product:
README
- Mock-up looks good
User-guide
- looks good
- features not implemented yet should be clearly marked as Coming in v2.0
Developer guide
- You can remove Appendix A and G if you want
- {More to be added} can be removed if no longer applicable
- Consider adding more NFRs
- Still a lot of AddressBook references, which should be refactor or removed.

eugenepeh · 2018-10-08T09:14:46Z

src/test/java/seedu/address/TestApp.java

-import seedu.address.model.ModelManager;
-import seedu.address.model.ReadOnlyAddressBook;
-import seedu.address.model.UserPrefs;
+import seedu.address.model.*;


remember to disable intellij auto import wildcard
Why is wild card import bad?

Also, the checkstyle check in travis will fail your CI test if you use import wildcard.

eugenepeh · 2018-10-08T09:16:22Z

build.gradle

@@ -66,6 +66,8 @@ dependencies {
    implementation group: 'com.sun.xml.bind', name: 'jaxb-impl', version: '2.3.0'
    implementation group: 'com.sun.xml.bind', name: 'jaxb-core', version: '2.3.0'
    implementation group: 'javax.activation', name: 'activation', version: '1.1.1'
+    //Import opencsv dependency to access csv file
+    compile group: 'com.opencsv', name: 'opencsv', version: '4.1'


compile is actually the syntax of old api, can change to implementation if possible.

eugenepeh · 2018-10-22T03:20:46Z

Avoid closing the PR abruptly unless you guys coming up with a new one

eugenepeh · 2018-10-22T13:48:17Z

@shuiherng @pratyushghosh @kumuwu @lixinze777 @ZZG229

Feedback for v1.2 T12-2

Please refer to my v1.1 comments for some of the things that you might have missed out.

Landing page

As highlighted, the nav bar can be updated as it doesn't applies to your project. Also, badges should be updated accordingly to reflect your repo's status.

Developer Guide

Can include some images and diagrams to explain your feature. Also, do try to come up with your design considerations.
Diagram not properly cropped. In power point you can actually select the shapes and save as image.
Interesting feature, what happen to the conjunction words? Also, can highlight your design considerations and limitations.
Abit TLDR because there seems to be more decisions etc under each steps, consider using bullet points to clean up a bit. Also, can use diagrams to project your explanation better.

Overall

Do get travis up and running soon.
Also, remember to work on making the product user-testable

eugenepeh · 2018-11-09T16:46:26Z

Some quick feedback for DG at mid-v1.4:

Should point to itself

Also, do check out my previous feedback to ensure that all is taken care of.
Lastly, you guys might want to use diagrams to better explain each of your features

Appointment card test

Allow prompt window to ding ding ding when trying to access main window

some minor change

…into natural-language

boost coverage

PPP

add code snippet in dg

…into natural-language

add two lines

remove unnecessary stuff in AboutUs

Project Portfolio Added & Licenses Added to README

ppp

rename the ppp file

nus-se-pr-bot added the team.2 label Sep 19, 2018

nus-se-pr-bot requested a review from eugenepeh September 19, 2018 18:24

nus-se-pr-bot assigned eugenepeh Sep 19, 2018

nus-se-pr-bot added the tutorial.T12 label Sep 19, 2018

eugenepeh reviewed Sep 24, 2018

View reviewed changes

eugenepeh added the Mid-v1.1-Reviewed label Sep 24, 2018

eugenepeh reviewed Oct 8, 2018

View reviewed changes

eugenepeh added the v1.1-Reviewed label Oct 8, 2018

shuiherng force-pushed the master branch from 2f4096c to 7fe11a0 Compare October 14, 2018 10:19

pratyushghosh closed this Oct 17, 2018

eugenepeh reopened this Oct 22, 2018

nus-cs2103-AY1819S1 deleted a comment from shuiherng Oct 22, 2018

eugenepeh added the v1.2-Reviewed label Oct 22, 2018

change sequence of methods

665cf56

eugenepeh added the Mid-v1.4-Reviewed label Nov 9, 2018

lixinze777 and others added 10 commits November 10, 2018 01:12

Merge pull request #158 from lixinze777/master

ea95b8c

Appointment card test

more test code

cdd3469

typo

8bd0b8e

change exception message and related tests

312bc91

Allow prompt window to ding ding ding when trying to access main window

144c22c

Travis

b495d97

change some test cases

a0fff31

Merge pull request #161 from lixinze777/master

212759f

Allow prompt window to ding ding ding when trying to access main window

Merge pull request #160 from kumuwu/natural-language

9972fef

some minor change

edit the style

73ceb82

kumuwu and others added 30 commits November 12, 2018 22:09

Merge branch 'master' of https://github.com/CS2103-AY1819S1-T12-2/main …

d022508

…into natural-language

Wrap up

eb7dfdb

boost coverage

2553ba0

Wrap up

0233bd1

Merge branch 'master' of https://github.com/CS2103-AY1819S1-T12-2/main

5a92e02

Merge pull request #209 from ZZG229/diseaseAndSymptom

7fa546c

boost coverage

add code snippet in dg

32f9859

Merge pull request #210 from lixinze777/master

01fa7d2

PPP

edit aboutUs

3fffba4

Merge pull request #211 from ZZG229/diseaseAndSymptom

1545889

add code snippet in dg

Merge branch 'master' of https://github.com/CS2103-AY1819S1-T12-2/main …

9019701

…into natural-language

sync DG with PPP

0f4934d

add two lines

7d1344b

Merge pull request #212 from ZZG229/diseaseAndSymptom

67de244

add two lines

ppp

d2df759

Merge branch 'master' of https://github.com/CS2103-AY1819S1-T12-2/main

95a6794

improve relevance of DG

c427aa1

change one line

7964fdd

Merge pull request #208 from kumuwu/natural-language

943c80f

remove unnecessary stuff in AboutUs

Project Portfolio Added

bd13754

Licenses Added To Readme

f2e02d4

Merge pull request #213 from pratyushghosh/master

876abb7

Project Portfolio Added & Licenses Added to README

PPP

4ca25ec

travis

ec6c352

Merge branch 'master' of https://github.com/CS2103-AY1819S1-T12-2/main

9a7fbd8

dg

cb6ddca

Merge pull request #214 from lixinze777/master

85e2b87

ppp

rename the file

647f9fb

delete the original file

d165ae8

Merge pull request #215 from ZZG229/diseaseAndSymptom

1712f60

rename the ppp file

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[T12-2] PatientBook #61

[T12-2] PatientBook #61

pratyushghosh commented Sep 19, 2018

eugenepeh left a comment •

edited

Loading

eugenepeh Sep 24, 2018

eugenepeh Sep 24, 2018

eugenepeh Sep 24, 2018

eugenepeh Sep 24, 2018

eugenepeh Sep 24, 2018

eugenepeh Sep 24, 2018

damithc commented Oct 2, 2018

eugenepeh commented Oct 8, 2018

eugenepeh left a comment •

edited

Loading

eugenepeh Oct 8, 2018

eugenepeh Oct 8, 2018

eugenepeh Oct 8, 2018

eugenepeh commented Oct 22, 2018

eugenepeh commented Oct 22, 2018

eugenepeh commented Nov 9, 2018

[T12-2] PatientBook #61

Are you sure you want to change the base?

[T12-2] PatientBook #61

Conversation

pratyushghosh commented Sep 19, 2018

eugenepeh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damithc commented Oct 2, 2018

eugenepeh commented Oct 8, 2018

eugenepeh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eugenepeh commented Oct 22, 2018

eugenepeh commented Oct 22, 2018

Feedback for v1.2 T12-2

Landing page

Developer Guide

Overall

eugenepeh commented Nov 9, 2018

eugenepeh left a comment •

edited

Loading

eugenepeh left a comment •

edited

Loading