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

[WIP] Implement api services #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Implement api services #9

wants to merge 3 commits into from

Conversation

vedi
Copy link

@vedi vedi commented Dec 26, 2019

No description provided.

@matin
Copy link
Contributor

matin commented Jan 11, 2020

@vedi does make test succeed?

@matin
Copy link
Contributor

matin commented Jan 14, 2020

@vedi what’s the intention of the refactoring? The code is working perfectly fine the way it is now. It looks like you’re trying to model the python library against the js library that you have, but the code in this branch is not at all pythonic.

If you do merge a refactor like this, we’d just go back to using our own version. It’s working in production today and is pythonic, which makes it easier to maintain. The changes you have here would also require that we make major changes to our systems to support the breaking changes that you’re introducing. So again, what’s the advantage of refactoring something that’s already well-written, pythonic, 100% tested, and working in production?

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