Skip to content


Subversion checkout URL

You can clone with
Download ZIP


Refactor #75

sigmavirus24 opened this Issue · 13 comments

3 participants


The library is pretty damn stable (in my humble opinion) and I'm annoyed personally by having some of the huge files that we do, e.g., github3/ I'm going to start splitting them off into sub-modules, e.g., github3/repos/{repo,repocommit,...}.py.

Thanks to code coverage and the unittests I can be certain I won't break anything with this refactor. ;)

On a separate note, I'm wondering if there is anything besides what issues are open that people might be interested in seeing included. While refactoring, I think I'm going to take care of #63 and #69 myself.

I think subclassing JSONEncoder is over-kill provided we have the {to,from}_json methods.

I'll probably echo this to the mailing list that I've all but neglected, in case there's anyone there with a different opinion. (Assuming there's anyone there at all :))


Also, I should explain that I'm not planning on breaking any of the API with this change. So this won't be a new semantic version being released as a result. I hope to squeeze this all into 0.6.0


I have a project which heavily depends on I once experienced upgrading from 0.1a8 to 0.5.1 on that project (see crosspop/asuka@1d7b6ab), and personally it’s not very hard for me. I think small API changes are acceptable as long as changelog is well provided. (It really helped my previous upgrading.)

Thank you for this great project. :-)


Thanks for the feedback @dahlia

And FYI, has status support (and has for a while now) so crosspop/asuka#2 should be simple to work out. :)

As a separate question, I see you're using some asynchronous methods elsewhere in your code. Would an asynchronous be worthwhile to you?


@sigmavirus24 It’s unnecessary for me, but others might need it. Why I don’t need it is that I’ve used coroutine-based implicit asynchronous networking library. There are libraries of this kind e.g. gevent, eventlet. Those two libraries patch (monkey patching) whole network stack of Python runtime, and it makes asynchronous as well. :wink:


Awesome. In that case I'll just point anyone who may in the future ask to this issue. :)

One other idea that just struck me, pengwynn/octokit has discussed giving users the ability to cache results. Would this be of interest/use? For the record, I'm thinking of using composition to achieve that if at all, i.e., the user builds/customizes a cache object and gives it to us to use with a specific method, etc.


I think the best way to achieve caching system is to provide an interface for it e.g. CacheBackend with default implementations of it e.g. NullCacheBackend, DictCacheBackend, FileCacheBackend.


Plus, invalidation is always a key problem of caching. Do you have any idea about this?


This would have been much easier had kennethreitz/requests#700 happened. Also, as mentioned on IRC, I would like an API akin to elarson / httpcache. If we did do something like this, I'm wondering if composition might not be the best API, i.e., tells you it wants an object to use as a cache with get, set and other methods and you provide it. Perhaps also providing a recipe for a reasonable cache in the docs would allow for this to be an unobtrusive change. :)


On that topic I should probably ping @ionrock about his opinions on this too as the author of httpcache


Also, I reserve the right to break out topics into other issues for my own sanity. :wink:


So I think there are some modules that people could argue should be broken up (orgs, users, pulls, models) but they are all < 500 lines long and while I may in the future split them up, right now I'm going to leave them as is. The really bad ones (issues, repos, gists) are all taken care of. I'm going to split the caching discussion out into a separate issue and merge @alejandrogomez's and finish it up so we can get 0.6.0 out the door for zci/zci (and @zyga)


@sigmavirus24 I won't have a lot of free time until the 4th of april, so if you want to speed things
up for a new release feel free to work on top of the few changes that I've made. I'm looking forward
to equality comparison support on the library.


Thanks for what you've done thus far @alejandrogomez . I'll probably push the last of the equality changes tonight and hopefully 0.6.0 by the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.