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

Fixes #472 #474 #476 Add News search in Baidu, Parsijoo & Mojeek #486

Merged
merged 1 commit into from
Jan 29, 2018

Conversation

bhaveshAn
Copy link
Member

Fixes #472
Fixes #474
Fixes #476
Addresses #468

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream master branch.
  • I have added necessary documentation (if appropriate)

Changes proposed in this pull request:

  • Added News search in Baidu, Parsijoo & Mojeek
  • Added tests for the same.

Providing Heroku deployment at https://young-dusk-37307.herokuapp.com/
@shankybora @rohitsainicool @gabru-md Please review. Thanks !!

@codecov
Copy link

codecov bot commented Jan 29, 2018

Codecov Report

Merging #486 into master will decrease coverage by 2.06%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
- Coverage   84.45%   82.39%   -2.07%     
==========================================
  Files          29       29              
  Lines         849      937      +88     
==========================================
+ Hits          717      772      +55     
- Misses        132      165      +33
Impacted Files Coverage Δ
test/test_baidu.py 100% <100%> (ø) ⬆️
app/scrapers/baidu.py 100% <100%> (ø) ⬆️
app/scrapers/parsijoo.py 100% <100%> (ø) ⬆️
test/test_parsijoo.py 100% <100%> (ø) ⬆️
app/scrapers/mojeek.py 100% <100%> (ø) ⬆️
test/test_mojeek.py 100% <100%> (ø) ⬆️
app/scrapers/__init__.py 82.85% <50%> (-4.24%) ⬇️
app/scrapers/generalized.py 38.88% <8.82%> (-9.3%) ⬇️

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 8a91095...b83c4b6. Read the comment docs.

Copy link

@rohitsainicool rohitsainicool left a comment

Choose a reason for hiding this comment

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

Great work again 💯

Copy link
Member

@ParthS007 ParthS007 left a comment

Choose a reason for hiding this comment

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

LGTM ! 💯

@ParthS007
Copy link
Member

@rohitsainicool @shankybora Please don't review without contributing and knowing about a project . You are welcome to contribute to FOSSASIA 👍

@bhaveshAn
Copy link
Member Author

Approval by a maintainer hence merging.

@bhaveshAn bhaveshAn merged commit 9e09370 into fossasia:master Jan 29, 2018
@bhaveshAn bhaveshAn deleted the news-feature branch January 29, 2018 17:53
@mariobehling
Copy link
Member

Please dont merge your own PRs.

@ParthS007
Copy link
Member

ParthS007 commented Jan 29, 2018

@mariobehling I and @vaibhavsingh97 have already asked him many times to follow best practices. We cannot rely on reviewers that are actually not contributing to project and @rohitsainicool & @shankybora are even new to Github.

@bhaveshAn
Copy link
Member Author

Okay @mariobehling @ParthS007

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.

Add News search support for Baidu Add News search support for Parsijoo Add News search support for Mojeek
5 participants