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

Update PPP #192

Merged
merged 14 commits into from
Nov 12, 2018
Merged

Update PPP #192

merged 14 commits into from
Nov 12, 2018

Conversation

jcjxwy
Copy link

@jcjxwy jcjxwy commented Nov 12, 2018

No description provided.

@coveralls
Copy link

coveralls commented Nov 12, 2018

Coverage Status

Coverage remained the same at 93.129% when pulling c5222e2 on jcjxwy:master into f893ed6 on CS2103-AY1819S1-T12-1:develop.

* **Alternative 1 (current choice):** Check through all expenses and select those with all the keywords
** Pros: Easy to implement. No need to change original architecture.
** Cons: Time-consuming. Tend to take longer time when there is a large number of expenses.
* **Alternative 2:** Store expenses in separated files and only check the relevant files while filtering.

Choose a reason for hiding this comment

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

separated should be separate

* **Alternative 1(current choice):** Filter out the targeted expenses and replace them with edited expenses.
** Pros: Easy to implement. Align with current architecture.
** Cons: Time-consuming. Tend to take longer time when there is a large number of expenses.
* **Alternative 2:** Store expenses in separated files. When the expenses are edited, move them

Choose a reason for hiding this comment

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

separated should be separate

* The order of the keywords does not matter, i.e. the KEYWORD `Have lunch` is equivalent to the KEYWORD `lunch have`
* Partial words will be matched, i.e. using the KEYWORD `Hav` for expense name will an expense with the name `Have`
* Partial words will be matched, i.e. using the KEYWORD `Hav` for expense name will give an expense with the name `Have`
* Only `Tag` keywords can have multiple keywords. `Name`/`Category`/`Cost`/`Date` can only have one keywords.

Choose a reason for hiding this comment

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

Maybe you can give an example of this? i think the user would be quite confused cause a keyword can have multiple words but only tag can have multiple keywords.

then... change the last part to... Name, Category, Cost and Date can only have one keyword

This section summarizes my contribution to the project.

* *Major enhancement:*
** added the ability to find and edit multiple expenses by specific keywords

Choose a reason for hiding this comment

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

added should be "Added"

@jonathantjm
Copy link

Also, make sure you rename your PPP to your github username and change the link in AboutUs.adoc accordingly

@@ -40,7 +40,7 @@ Responsibilities: UI

=== Jiang Chen
image::jcjxwy.png[width="150", align="left"]
{empty}[https://github.com/jcjxwy[github]] [<<jiangchen#, portfolio>>]
{empty}[https://github.com/jcjxwy[github]] [http://localhost:63342/expense%20tracker%201.2/expense_tracker_1.2/build/docs/html5/team/jcjxwy.html?_ijt=k195lrlr4suj08ud9qlr4i30ht[portfolio]]

Choose a reason for hiding this comment

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

umm i think you just need to change it to <<jcjxwy#, portfolio>>

@jonathantjm jonathantjm merged commit 912cb41 into CS2103-AY1819S1-T12-1:develop Nov 12, 2018
@JasonChong96 JasonChong96 modified the milestone: v1.4 Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants