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

[T9][F11-C2] #177

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

Conversation

njyjn
Copy link

@njyjn njyjn commented Oct 21, 2016

Ready for review.

njyjn and others added 30 commits October 6, 2016 23:46
AboutUs finalized. Merged by Justin.
v0.0.1 finalized. Merged by Justin.
Ready to merge. Please note that we use ToDoList in place of TaskManagert—I will manually cherrypick.
wengkiat and others added 27 commits October 20, 2016 13:06
Conflicts:
	src/main/java/seedu/cmdo/commons/core/Config.java
Copy link

@Candiie Candiie left a comment

Choose a reason for hiding this comment

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

Hi guys! Wow, I have a total of 30 review comments lols! I didn't even realized XD Anways do go through all of them and make the necessary changes and close Prs when u're done ~

P.S. Don't waste my efforts & Do go through them cos there are quite a few critical mistakes here and there ~
Jiayous!!!

@@ -113,7 +113,7 @@ The `UI` component,

**API** : [`Logic.java`](../src/main/java/seedu/address/logic/Logic.java)
Copy link

Choose a reason for hiding this comment

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

Hi Guys, its ok to use the existing diagrams, however do take note to update the diagram according to YOUR project's structure! e.g i still see post(AddressBook...) or address book related images everywhere, same for ur UI component, in your case are u sure you are using "PersonCard" ?

@@ -113,7 +113,7 @@ The `UI` component,

**API** : [`Logic.java`](../src/main/java/seedu/address/logic/Logic.java)

1. `Logic` uses the `Parser` class to parse the user command.
Copy link

Choose a reason for hiding this comment

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

Under UI API Java

  • The UI consists of a MainWindow that is made up of parts e.g.CommandBox, ResultDisplay, PersonListPanel
    ~ person ~
    do take note to go through all, because if this is seen in the final vetting ~ yupp we all know what will happen ><"

@@ -113,7 +113,7 @@ The `UI` component,

**API** : [`Logic.java`](../src/main/java/seedu/address/logic/Logic.java)

1. `Logic` uses the `Parser` class to parse the user command.
1. `Logic` uses the `MainParser` class to parse the user command. `MainParser` relies on `Parser` of [Natty by Joe Stelmach](https://github.com/joestelmach/natty) for natural language processing.
Copy link

Choose a reason for hiding this comment

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

For the logic diagram, i'm sure you guys have more then add, list commands, do add in a few more too since there's still spaces and do update the image to how your logic work in ur program :)

@@ -113,7 +113,7 @@ The `UI` component,

**API** : [`Logic.java`](../src/main/java/seedu/address/logic/Logic.java)

1. `Logic` uses the `Parser` class to parse the user command.
1. `Logic` uses the `MainParser` class to parse the user command. `MainParser` relies on `Parser` of [Natty by Joe Stelmach](https://github.com/joestelmach/natty) for natural language processing.
2. This results in a `Command` object which is executed by the `LogicManager`.
3. The command execution can affect the `Model` (e.g. adding a person) and/or raise events.
Copy link

Choose a reason for hiding this comment

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

"person here"
btw i'm not being naggy ok ~ since i'm already going through i'll try to mark those that I can find so that u guys have an easier life to do the neccessary changes ~ however, do note to not only change the names but also the explanation if neccessary ~ when we go through your logic codes, we expect it to run in a similar manner as explained

Copy link

Choose a reason for hiding this comment

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

or shown in ur diaggrams

@@ -126,8 +126,8 @@ The `UI` component,

The `Model`,
Copy link

Choose a reason for hiding this comment

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

Similar for the image here ~ I'm seeing Tag Person ~ Name phone addy, email ~

Copy link

Choose a reason for hiding this comment

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

U updated the text for Read Only Task below but not the image ~ so do take note of this too :)

5. Customize commands to suit user preference
6. Able to toggle layout for dark and light for colour blindness
7. Issue reminders for upcoming tasks
8. Auto-complete functionality for user entries
Copy link

Choose a reason for hiding this comment

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

hmmmmmmmmm....... this too i don't this this is a NFR ~ unless u are justifying it as "Efficiency?" do think about it and know how to justify your stand if u were questioned.

Copy link

Choose a reason for hiding this comment

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

Otherwise, I do suggest this to be taken aaway ~

6. Able to toggle layout for dark and light for colour blindness
7. Issue reminders for upcoming tasks
8. Auto-complete functionality for user entries
9. Google search function in CMDo

{More to be added}
Copy link

Choose a reason for hiding this comment

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

rmv this is u don't have any more to add ~ make sure ur User stories matches ur Use Cases~

@@ -300,11 +549,56 @@ Use case ends.

> Windows, Linux, Unix, OS-X

##### Private contact detail
## Appendix E : Product Survey
Copy link

Choose a reason for hiding this comment

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

Wow @team, in general I do like how u guys tried to survey the products with the approach of how you can compare it to the features/functionalities in the app you are building! Great Job! This is how product surveys should be done!!!

However since this is to be added into the report I do urge you guys to add a few more observations ~

Copy link

Choose a reason for hiding this comment

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

bcos ~ although it may not seem so, product evaluation is one of the most important part of software engineering ~ you learn from other peoples' failures and can try to improve your app by not making the same mistakes

4. Can share with another person via email.

**CONS**
1.
Copy link

Choose a reason for hiding this comment

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

really? no cons? if i can list one r u gonna do 100 push up for me :)

Redo the earlier action.
Format: `redo`

#### Change the storage location : `storage`
Copy link

Choose a reason for hiding this comment

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

do rmb to update ur user guide too if needed along side with ur implmentations~ great job updating the block and find :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants