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

updating README to fit with the API #22

Merged
merged 1 commit into from Dec 4, 2015

Conversation

stummjr
Copy link

@stummjr stummjr commented Dec 4, 2015

I used the API and noticed that it behaves slightly different from the documentation. So, this PR is meant to update the documentation.

Summary of changes:

  • project_ids() returns a list of integers instead of a list of strings
  • project.spiders() returns a list of dicts with spider metadata instead of a list with the spider names
  • schedule and project.id return an id like 123/2/12 rather than an hexa string


And select a particular project to work with::

>>> project = conn['123']
>>> project = conn[123]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think both versions work. :)

Copy link
Author

Choose a reason for hiding this comment

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

@eliasdorneles they do work, but since the conn.project_ids() returns a list of int, it looks more consistent if we use conn[123] rather than conn['123'] in the doc.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough!
LGTM, merging!

Copy link
Contributor

Choose a reason for hiding this comment

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

I would have left an example using string since the most common task is passing the project id from the arguments of a script.

eliasdorneles added a commit that referenced this pull request Dec 4, 2015
updating README to fit with the API
@eliasdorneles eliasdorneles merged commit 8bd4137 into scrapinghub:master Dec 4, 2015
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