search

micro-fan / aiozk

Asyncio client for zookeeper
MIT License
49 stars 20 forks source link

Refactoring for v1.0 #67

Open cybergrind opened 4 years ago

cybergrind commented 4 years ago

Since we've started to break API's it is probably a good time to make more changes. Let's get our ideas in issues and then split and code them.

https://github.com/micro-fan/aiozk/projects/1

cybergrind commented 4 years ago

@rhdxmr I believe we can get rid of Recipe.sub_recipes: the only function of it that I can see is to facilitate the propagation of client in Recipe.set_client and probably can be done better:

  1. introspection of __dir__ contents for isinstance(Recipe). Won't require many changes to do
  2. We can add an optional parent parameters to the construction. Will require more changes but makes it explicit

What do you think?

rhdxmr commented 4 years ago

@rhdxmr I believe we can get rid of Recipe.sub_recipes: the only function of it that I can see is to facilitate the propagation of client in Recipe.set_client and probably can be done better:

  1. introspection of __dir__ contents for isinstance(Recipe). Won't require many changes to do
  2. We can add an optional parent parameters to the construction. Will require more changes but makes it explicit

What do you think?

I don't understand your first suggestion. I am sorry. How can we use __dir__ instead of .sub_recipes? And what does __dir__ have to do with isinstance(Recipe)?

And for the second suggestion, I guess you mean adding parent parameter to sub recipes. Is it right?

rhdxmr commented 4 years ago

Connection.abort method seems WIP. It should be improved.

rhdxmr commented 4 years ago

To release the 1.0.0 version, we should ensure that aiozk is stable enough. So I think we need to make up for test code and keep test code coverage stats high.

Besides documents should be completed for normal users.

It would be good to create other issues for these must-do's.

cybergrind commented 4 years ago

So I think we need to make up for test code and keep test code coverage stats high.

yeah, already put it to the list