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

Refactoring #7

Closed
wants to merge 21 commits into from
Closed

Refactoring #7

wants to merge 21 commits into from

Conversation

hanpeter
Copy link
Contributor

@hanpeter hanpeter commented Sep 23, 2016 •
edited
Loading

What does this PR do?

This PR refactors following parts of the library:

  1. The logging is now done with logging modules.
  2. The methods leading up to and following request call are cleaned up.
  3. The credentials are no longer required for every call.
  4. The authentication token is refreshed whenever needed.
  5. Docstrings are cleaned up.
  6. cli.py is added to provide a quick and dirty way to test the calls.
  7. 'DateTime` library is removed from the requirements as it is no longer needed.

Where should the reviewer start?

catchpoint.py

How was this tested? How can the reviewer verify your testing?

The change was manually tested on development environment using cli.py.

NOTE:

  • The docstring for _format_time() is not updated and sticks out like a sore thumb. I am planning to update that method to be more robust, but haven't come up with a perfect way yet.

hanpeter and others added 21 commits September 12, 2016 14:09
@hanpeter
Minor string format change for readability
26680d3
@hanpeter
Logging via logging.Logger rather than sys.stderr.
7cbfb6f
@hanpeter
Merge pull request #1 from krux/phan-cleanup-setup
43cf414 
Clean up setup.py
@hanpeter
URL for requests are formatted in helper method
7a6ab50
@hanpeter
Testing CLI now runs all requests
619e10d
@hanpeter
Adding a use case where no parameters are passed to test CLI
34e93f5
@hanpeter
Cleaning up _make_request()
b53ebda
@hanpeter
Cleaning up authentication process
c735b04
@hanpeter
Merge pull request #2 from krux/phan-decorators
1e1985b 
Logger and request refactoring
@hanpeter
Strip out unnecessary DateTime requirement
44fdd28
@hanpeter
Removing client ID and secret
84f5af8
@hanpeter
Cleaning up the comment
6633db3
@hanpeter
del is more explicit than pop()
a5badd6
@hanpeter
Fixing for code review points
a97bb8a
@hanpeter
Some docstring updates
91517f5
@hanpeter
Merge pull request #3 from krux/phan-auth
bbe6eee 
Authetication cleanup
@hanpeter
Docstring bonanza
7928a66
@hanpeter
Updating README to reflect the changes made
730d5b1
@hanpeter
Removing _debug() method
9eaa032
@hanpeter
Merge branch 'phan-docstring' of github.com:krux/catchpoint-api-pytho…
22535b0 
…n into phan-docstring
@hanpeter
Merge pull request #4 from krux/phan-docstring
89faadd 
Documentation update
@hanpeter
Copy link
Contributor Author

hanpeter commented Oct 1, 2016

@jasonarewhy Any way I can speed up the review process to get this PR merged?

@amanuensis
Copy link

This was actually really good thanks I was able to use this a personal project.

@hanpeter hanpeter closed this Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hanpeter @amanuensis