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

Issue #114 - API endpoint for setting cookies #116

Merged
merged 29 commits into from
Dec 14, 2017
Merged

Issue #114 - API endpoint for setting cookies #116

merged 29 commits into from
Dec 14, 2017

Conversation

anudeepti2004
Copy link
Collaborator

@anudeepti2004 anudeepti2004 commented Aug 21, 2017 •
edited by aecio
Loading

Fixes issue #114.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 45.649% when pulling 434f25e on cookies into 182b55f on master.

@anudeepti2004
Fixed bugs related to NullPointerException in SimpleHttpFetcher and e…
5f3247c 
…rror messages regarding adding cookies in CrawlerResource
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 45.651% when pulling 5f3247c on cookies into 182b55f on master.

@aecio aecio changed the title Cookies #114 - API endopoin for setting cookies Aug 21, 2017
@aecio aecio changed the title #114 - API endopoin for setting cookies #114 - API endopoint for setting cookies Aug 21, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 46.059% when pulling 1726fe6 on cookies into 182b55f on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 46.192% when pulling 782b40f on cookies into 182b55f on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 46.785% when pulling 0441759 on cookies into 182b55f on master.

@anudeepti2004 anudeepti2004 mentioned this pull request Sep 12, 2017
Closed
@aecio
Merge branch 'master' into cookies
8cdd9fa 
src/main/java/focusedCrawler/crawler/async/fetcher/FetcherFactory.java
src/main/java/focusedCrawler/crawler/async/fetcher/OkHttpFetcher.java
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 47.406% when pulling 8cdd9fa on cookies into fcfa245 on master.

@aecio
Finish merge and refactoring
e37b396
@aecio
Merge branch 'master' into cookies
2b80ed0 
src/main/java/focusedCrawler/crawler/async/fetcher/OkHttpFetcher.java
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 53.661% when pulling 2b80ed0 on cookies into da25b33 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 53.41% when pulling bcaf987 on cookies into da25b33 on master.

aecio
aecio requested changes Nov 17, 2017
View reviewed changes
Copy link
Member

@aecio aecio left a comment

Choose a reason for hiding this comment

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

Overall, looks good. I added some minor notes. Thanks.

src/main/java/focusedCrawler/crawler/cookies/OkHttpCookieJar.java Outdated

allCookies.addAll(loadValidCookies(url1));

if (strUrl.substring(i + 1).equals(url.topPrivateDomain())) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know how expensive url.topPrivateDomain() is , so it is better to store the value in a local variable.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 53.774% when pulling d4de82c on cookies into 063faf7 on master.

@aecio aecio changed the title #114 - API endopoint for setting cookies Issue #114 - API endopoint for setting cookies Dec 14, 2017
@aecio aecio changed the title Issue #114 - API endopoint for setting cookies Issue #114 - API endpoint for setting cookies Dec 14, 2017
@aecio aecio merged commit 8e3a023 into master Dec 14, 2017
@aecio aecio deleted the cookies branch June 1, 2018 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aecio aecio aecio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@anudeepti2004 @coveralls @aecio @maqzi