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

Create c-tag-find/c-today/c-tomorrow feature #141

Merged
merged 8 commits into from
Oct 28, 2020

Conversation

yanbingtao
Copy link

@yanbingtao yanbingtao commented Oct 27, 2020

  1. c-today:
  • Able to list down all persons whose tag(s) match today's day (i.e. Monday, Tuesday, etc).
  1. c-tomorrow:
  • Able to list down all persons whose tag(s) match the next day's day (i.e. Monday, Tuesday, etc).
  1. c-tag-find KEYWORD [KEYWORDS] ...
  • Able to list down all persons whose tag(s) match the keyword(s) entered by the user.

@yanbingtao yanbingtao changed the title Creates c-tag-find feature Create c-tag-find feature Oct 27, 2020
@codecov-io
Copy link

codecov-io commented Oct 27, 2020

Codecov Report

Merging #141 into master will decrease coverage by 2.10%.
The diff coverage is 7.07%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #141      +/-   ##
============================================
- Coverage     63.57%   61.46%   -2.11%     
  Complexity      665      665              
============================================
  Files           119      124       +5     
  Lines          2380     2473      +93     
  Branches        287      294       +7     
============================================
+ Hits           1513     1520       +7     
- Misses          755      841      +86     
  Partials        112      112              
Impacted Files Coverage Δ Complexity Δ
...main/java/seedu/address/commons/core/Messages.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...seedu/address/logic/commands/FindByTagCommand.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../address/logic/commands/FindByTagTodayCommand.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...dress/logic/commands/FindByTagTomorrowCommand.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../seedu/address/logic/parser/AddressBookParser.java 61.76% <0.00%> (-5.98%) 17.00 <0.00> (ø)
...edu/address/logic/parser/TagFindCommandParser.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ess/model/person/TagContainsKeywordsPredicate.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../java/seedu/address/model/util/SampleDataUtil.java 12.00% <0.00%> (ø) 1.00 <0.00> (ø)
src/main/java/seedu/address/model/Model.java 55.00% <50.00%> (-11.67%) 4.00 <0.00> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a759a03...140892d. Read the comment docs.

Copy link

@ureshiiYing ureshiiYing left a comment

Choose a reason for hiding this comment

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

Other comments: Perhaps naming the commands as TodayCommand and TomorrowCommand isn't the best? It felt vague to me. The missing javadoc descriptions did not help too.

@yanbingtao
Copy link
Author

Other comments: Perhaps naming the commands as TodayCommand and TomorrowCommand isn't the best? It felt vague to me. The missing javadoc descriptions did not help too.

I've changed these two to FindByTagTodayCommand.java and FindByTagTomorrowCommand.java. And two Javadoc were also added for these two classes to better explain the meaning. Thanks!

@yanbingtao yanbingtao changed the title Create c-tag-find feature Create c-tag-find/c-today/c-tomorrow feature Oct 28, 2020
Diwu-Yi
Diwu-Yi previously approved these changes Oct 28, 2020
Copy link

@Diwu-Yi Diwu-Yi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@Diwu-Yi Diwu-Yi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@Persdre Persdre left a comment

Choose a reason for hiding this comment

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

LGTM

@Persdre Persdre merged commit e390bc8 into master Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.Medium Nice to have type.Story A user story
Projects
None yet
5 participants