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

Patents api #16

Closed
wants to merge 15 commits into from
Closed

Patents api #16

wants to merge 15 commits into from

Conversation

jeorryb
Copy link

@jeorryb jeorryb commented Dec 8, 2016

I wanted to see if I was on the right track. I removed explicit paramaters and have them defined in a config file. I also created a class for Patent. I also removed the function that is performing argument parsing since that is being handled elsewhere. Thank you!

@Mierdin
Copy link
Contributor

Mierdin commented Dec 8, 2016

Two things:

  1. What are your plans for Adding bindings for the "Patents" API #13?
  2. Is this a WIP PR or is it ready for a review?

@jeorryb
Copy link
Author

jeorryb commented Dec 9, 2016

I was think of output to a file if an argument is present in the Params. I'll work on adding that tomorrow as well as unit tests for the API. Can we start adding parameters in the sub-argument section of commands.py for our relevant API?

@jeorryb
Copy link
Author

jeorryb commented Dec 9, 2016

Still WIP, wanted some feedback if I was headed in the right direction.

@Mierdin
Copy link
Contributor

Mierdin commented Dec 9, 2016

At this point I would just focus on writing a class/functions for the Patents API only. Don't worry about the CLI or commands. In terms of testing this functionality, that can/should be done by a unit test anyways, so you might want to include that. Again, think of this as just a part of a library. Ignore for now the fact that this will be used by a CLI, because the reality is it may not always. We want to design this part of the library to be accessible to both the CLI package we'll create later, as well as some other 3rd party Python code.

EDIT: Accidentally hit the close button....reopening

@Mierdin Mierdin closed this Dec 9, 2016
@Mierdin Mierdin reopened this Dec 9, 2016
@Mierdin
Copy link
Contributor

Mierdin commented Dec 19, 2016

@jeorryb What's the state of this PR? Ready for me to take a peek? Also, any relation to #13?

@jeorryb
Copy link
Author

jeorryb commented Dec 19, 2016

Almost done with the unit tests. Slow going right now since I'm wrapping my head around mock and unittest. Hopefully I'll be able to submit the PR by end of day.

@Mierdin
Copy link
Contributor

Mierdin commented Dec 19, 2016

@jeorryb Great news! Don't hesitate to ask for help, I remember how difficult mock was when I first learned it.

add unit tests for patent.py
@jeorryb
Copy link
Author

jeorryb commented Dec 21, 2016 via email

@jeorryb jeorryb closed this Dec 22, 2016
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