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.

Sign up for GitHub

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

Jump to bottom

Search Context #107

Merged
merged 16 commits into from
Apr 2, 2019
Merged

Search Context #107

merged 16 commits into from
Apr 2, 2019

Conversation

qjqqyy
Copy link

@qjqqyy qjqqyy commented Mar 27, 2019 •
edited

This PR builds on top of #106 add-archives.
Part of #25.

Summary of changes

@qjqqyy qjqqyy added this to the v1.3 milestone Mar 27, 2019
@qjqqyy qjqqyy changed the title ViewFeed Context & Feed Management Search Context & Feed Management Mar 27, 2019
@thomastanck
Copy link

Is it possible to split this into 3-4 PRs?

@qjqqyy
Copy link
Author

qjqqyy commented Mar 31, 2019 •
edited

Add new context for viewing feed (resolves #26)
Strip HTML tags from description field of the RSS feed when viewing.
bing command searches bing for keywords (resolves #109)
Implement feed management (#69) (WIP)

Is it possible to split this into 3-4 PRs?

1 and 3 are closely related, while 2 is a minor enhancement. Maybe I can extract the last one into a separate PR?

@qjqqyy qjqqyy marked this pull request as ready for review March 31, 2019 19:50
@coveralls
Copy link

coveralls commented Mar 31, 2019 •
edited

Pull Request Test Coverage Report for Build 509

  • 62 of 106 (58.49%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.5%) to 88.157%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/seedu/address/logic/parser/EntryBookParser.java 1 2 50.0%
src/main/java/seedu/address/model/EntryBook.java 10 11 90.91%
src/main/java/seedu/address/logic/commands/BingWebSearchCommand.java 0 3 0.0%
src/main/java/seedu/address/commons/util/FeedUtil.java 27 32 84.38%
src/main/java/seedu/address/logic/parser/BingWebSearchCommandParser.java 0 6 0.0%
src/main/java/seedu/address/logic/parser/IndexedAddCommandParser.java 0 6 0.0%
src/main/java/seedu/address/logic/parser/EntryBookSearchParser.java 1 11 9.09%
src/main/java/seedu/address/logic/commands/IndexedAddCommand.java 0 12 0.0%
Totals Coverage Status
Change from base Build 507: -1.5%
Covered Lines: 1995
Relevant Lines: 2263
💛 - Coveralls

@thomastanck
Copy link

1 and 3 are closely related, while 2 is a minor enhancement. Maybe I can extract the last one into a separate PR?

Yep, sure! Then since 1-3 are done after extracting we can probably merge this in quickly.

@qjqqyy qjqqyy changed the title Search Context & Feed Management Search Context Apr 1, 2019
@qjqqyy qjqqyy requested a review from epicfailname April 1, 2019 17:27
@qjqqyy qjqqyy mentioned this pull request Apr 1, 2019
Merged
4 tasks
thomastanck
thomastanck approved these changes Apr 2, 2019
View reviewed changes
Copy link

@thomastanck thomastanck left a comment

Choose a reason for hiding this comment

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

LGTM

@thomastanck thomastanck dismissed epicfailname’s stale review April 2, 2019 10:17

Changes have been addressed.

@qjqqyy qjqqyy merged commit 40d661e into master Apr 2, 2019
@qjqqyy qjqqyy deleted the feeds-stuff branch April 2, 2019 10:20
@rlrh rlrh mentioned this pull request Apr 2, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomastanck thomastanck thomastanck approved these changes

@epicfailname epicfailname Awaiting requested review from epicfailname

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.

As a user, I can search the web As a frequent user, I want to add links from feeds
4 participants
@qjqqyy @thomastanck @coveralls @epicfailname