Skip to content

Conversation

@ashack293
Copy link
Contributor

This pull requests adds an optional starting index for google requests. You can set the number of results by which you want your query to be offset. To use it, change syntax from:

google ('node.js best practices',  callback) 

to:

google ('node.js best practices',  10, callback) 

@ashack293 ashack293 mentioned this pull request Jun 24, 2015
@ashack293
Copy link
Contributor Author

Is there any way to rerun the Travis build? I'm not sure that the failure was related to my changes.

@ashack293
Copy link
Contributor Author

Here are my local test results:

 + google()
    ✓ should return search results (1405ms)
    when resultsPerPage is set
      ✓ should return search results (625ms)
    when timeSpan is set
      ✓ each time-based query should return search results (463ms)
    when nextText and lang are set
      ✓ should return next page search results (1694ms)

  4 passing (4s)

@jprichardson
Copy link
Owner

Paging is already supported. What's the use case for not wanting to start on the first page?

@ashack293
Copy link
Contributor Author

My use-case is an application running on multiple instances which needs to add a batch of results to a database independent of the state of other instances, and independent of the application lifecycle on a single instance.

Making the paging an optional parameter with a few lines of code made a lot more sense to me than having to build state management around the module. This change makes it easy to check a database for the index of the last page retrieved from Google, and run the next query from there.

As an aside, it is easier to use the module without abusing it if you do a deeper (multi-page) search over a longer period of time instead of paging on a loop.

@jprichardson
Copy link
Owner

As an aside, it is easier to use the module without abusing it if you do a deeper (multi-page) search over a longer period of time instead of paging on a loop.

This is an excellent point. Ok, I'll incorporate this... I think that I wanna refactor the module...

@jprichardson
Copy link
Owner

I'll accept the PR, but first, please add a test. Something as simple as searching for something with the new parameter and checking for results.

@ashack293
Copy link
Contributor Author

Great! Test added.

jprichardson added a commit that referenced this pull request Jul 3, 2015
added optional start parameter, enabling pagination
@jprichardson jprichardson merged commit bc746ac into jprichardson:master Jul 3, 2015
@jprichardson
Copy link
Owner

Thanks, published :)

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.

2 participants