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

[T7][T15-C4] #43

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

Conversation

JamesHuangUC
Copy link

Ready for review

Copy link

@nishantbudhdev nishantbudhdev left a comment

Choose a reason for hiding this comment

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

Some comments added. Please review them and close the PR.

@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<projectDescription>
<name>addressbook-level4</name>
<comment>Project addressbook-level4 created by Buildship.</comment>
<name>main</name>

Choose a reason for hiding this comment

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

Add your project name here.

<img src="images/YouLiang.jpg" width="150"><br>
Role: Developer <br>
Responsibilities: UI
Responsibilities: Documentation, Testing, Deliverables and Deadlines

Choose a reason for hiding this comment

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

You can also mention the packages each person is in-charge of in this file. If not, I hope you have already divided it and have an understanding of the same among yourself.

`* *` | user | auto-complete certain tasks | save time instead of having to type out the whole command.
`*` | user | manipulate the schedule with a mouse | save time manipulating the calendar directly as opposed to having to go back to the CLI and type in more command rescheduling events (e.g. drag-drop vs. retyping event details)
`*` | user | view list or calendar in a pop out window | look at it more easily while working on other items.
`*` | user | have a nice GUI | have an easier time manipulating the information and let the program be more pleasing to my eyes.

Choose a reason for hiding this comment

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

Sounds more like a non-functional requirement. Try adding more specific details about the feature.

`* *` | user | specify the location to place the data storage | do things with it, like sync it to my Dropbox.
`* *` | user | bring up the program with a hotkey | pull it up quickly and conveniently when I need to add a task.
`* *` | user | schedule multiple time blocks with one task | schedule tasks or events that have multiple and different deadlines or dates.
`* *` | user | auto-complete certain tasks | save time instead of having to type out the whole command.

Choose a reason for hiding this comment

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

Define certain ? Mention the type of tasks (deadlines, events or floating tasks) you wish to to auto-complete.

`* * *` | user | receive reminders for upcoming tasks | don't forget about tasks
`* *` | user | add tasks through plain English | type more naturally than having to write in commands and flags.
`* *` | user | view tasks in a calendar format | figure out what events/tasks I have upcoming more easily.
`* *` | user | specify the location to place the data storage | do things with it, like sync it to my Dropbox.
Copy link

@nishantbudhdev nishantbudhdev Oct 10, 2016

Choose a reason for hiding this comment

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

Ah, this is a must-have feature. It needs to be assigned higher priority.

4. Should favor DOS style commands over Unix-style commands.
1. Program should load within 5 seconds
2. Storage file should be limited to 100MB default (can be changed by user)
3. Should work on any mainstream OS

Choose a reason for hiding this comment

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

Mention the maintstream OS you are aiming for.

Copy link

Choose a reason for hiding this comment

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

We defined the term 'mainstream OS' in the glossary; is that enough, or should we mention them specifically in the NFR?

4. Should hold up to 1000 tasks/events on the active task list at any time
5. Comes with automated unit tests.
6. Commands should be intuitive and easy to use.
7. Interface is simple and easy to understand.

Choose a reason for hiding this comment

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

Can add simple and easy to understand for who ? Beginner, Novice, Expert, Jim ?

4. Type the command in the command box and press <kbd>Enter</kbd> to execute it. <br>
e.g. typing **`help`** and pressing <kbd>Enter</kbd> will open the help window.

1. Download the latest '[insert program name here]' from the releases (../../../release) tab

Choose a reason for hiding this comment

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

'insert program name here' 😄

@@ -16,28 +16,28 @@
*/
public class AddressBook implements ReadOnlyAddressBook {

Choose a reason for hiding this comment

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

Please rename AddressBook.

*/
public class Deadline {

public static final String MESSAGE_DEADLINE_CONSTRAINTS =

Choose a reason for hiding this comment

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

Is public the best access modifier for MESSAGE_DEADLINE_CONSTRAINTS ? As a rule of thumb start all data members as private, and depending on their usage promote them to protected, and if absolutely necessary, to public.

@nishantbudhdev
Copy link

@JamesHuangUC @mchen14 I have some general remarks:

  • Avoid using public data members unless essentially required.
  • Update your Unit testing to reflect your new code.
  • Be on top of your integration testing and modifying the tests to suite your task manager.
  • Create issues for all your user stories and assign people to each one of them. This will allow us to easily see the amount of work each person has done. We will also look into the quality of code alongwith with the significance of the code within the complete project.
  • You can attach labels to prioritize your issues and you can add more issues for a complex user story once you start working on it.
  • Please mark your v0.2 of your project using labels before coming to lab this Thursday.

mchen14 and others added 6 commits October 10, 2016 15:04
updated guides according to feedback from mentor
…Murray branch. However, made new temp branch because will eventually go back to Murray branch once errors are discovered and it isnt the day before the deadline. Fixes #17
mchen14 and others added 28 commits November 3, 2016 14:53
updated developer guide and other small changes
Added and updated sort command, user guide, ui
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.

3 participants