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

Add integration for pylama #6

Closed
wants to merge 1 commit into from
Closed

Conversation

aequitas
Copy link

No description provided.

@myint
Copy link
Member

myint commented Nov 17, 2015

Thanks, but I think this would be better as a separate project. Currently, eradicate has no external dependencies, which I prefer.

@myint myint closed this Nov 17, 2015
@aequitas
Copy link
Author

I understand. But the dependency would already be fulfilled because you would have pylama installed in order to use the integration. Not having the pylama dependency does not break the normal functionality of this package, as the pylama_eradicate module is only parsed when it is invoked as a pylama linter through the entry_point.

Another solution might be to use 'extras' in order include pylama as a dependency but only if the package is installed with the pylama 'extra' explicitly like: pip install eradicate[pylama].

@myint
Copy link
Member

myint commented Nov 18, 2015

I think I prefer it as a separate project more due to logical dependency rather than what setup.py will install as a dependency. This script will become my responsibility as the owner of this package. Thus, I would feel the need to test it, which means writing test cases for it. I think it would be better if someone that actually uses pylama maintains such a plugin.

(By the way, I think entry_points requires setuptools, whereas this setup.py is currently using plain distutils.)

@aequitas
Copy link
Author

I did not pay attention to the setup.py implementation. I will create the plugin it as a separate repository or ask if the pylama dev want to integrate it in his package.

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