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

Mct/bing v7 #144

Merged
merged 14 commits into from
Dec 10, 2018
Merged

Mct/bing v7 #144

merged 14 commits into from
Dec 10, 2018

Conversation

MothOnMars
Copy link
Contributor

No description provided.

@@ -1,4 +1,4 @@
class AzureFormattedQuery < FormattedQuery
class BingFormattedQuery < FormattedQuery
DEFAULT_DOMAIN_SCOPE = 'site:gov OR site:mil'.freeze
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this replacing AzureFormattedQuery? I still see it in coverage/index.html line 43909.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like that's from a previous test run. If you nuke your coverage directory and re-run the specs, you should not see those old lines in coverage/index.html. (Searching with git grep should also ignore the coverage directory, as it's listed in .gitignore.)

@@ -1,4 +1,4 @@
class BingV6ImageResponseParser < BingV6ResponseParser
class BingImageResponseParser < BingResponseParser
def results
Copy link
Contributor

Choose a reason for hiding this comment

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

BingV6ImageResponseParser is also being referenced in : coverage/index.html line 43958 and 44265

@@ -1,4 +1,4 @@
class BingV5ResponseParser
class BingResponseParser
attr_reader :engine
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also being referenced in index.html

Copy link
Contributor

@peggles2 peggles2 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MothOnMars MothOnMars force-pushed the mct/bing_v7 branch 2 times, most recently from 291656c to ebe6d04 Compare October 9, 2018 16:48
@MothOnMars
Copy link
Contributor Author

@peggles2 , I added two commits yesterday to add some extra test coverage:
572352d
ebe6d04

Once you've had a chance to review & approve those commits, I will deploy for testing.

Copy link
Contributor

@peggles2 peggles2 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MothOnMars MothOnMars merged commit 9ab587e into master Dec 10, 2018
@MothOnMars MothOnMars deleted the mct/bing_v7 branch December 10, 2018 15:19
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

3 participants