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

Remove dead code. #313

Merged
merged 1 commit into from
Mar 17, 2018
Merged

Remove dead code. #313

merged 1 commit into from
Mar 17, 2018

Conversation

FGtatsuro
Copy link
Contributor

I found several dead code in neovim/api/common.py. This PR removes them.

Best regards.

@@ -154,10 +150,6 @@ def __contains__(self, item):
return item in self._fetch()


def _identity(obj, session, method, kind):
return obj
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for debugging I think

Copy link
Contributor Author

@FGtatsuro FGtatsuro Mar 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry. So, I remove this commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Force updated.

@@ -69,8 +69,8 @@ class RemoteMap(object):
It is used to provide a dict-like API to vim variables and options.
"""

def __init__(self, obj, get_method, set_method=None, self_obj=None):
"""Initialize a RemoteMap with session, getter/setter and self_obj."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is used or not, but it seems like there's no test coverage. If it is used a test would be nice.

Copy link
Contributor Author

@FGtatsuro FGtatsuro Mar 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your comment. As you say, I should add a test covering this change.
I'll add several tests after I read existing tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one test is enough. @bfredl would know better whether self_obj is still needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self_obj looks completely dead to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bfredl

Thank you for your check!

I've checked existing test code(under test directory), and I have a question.

RemoteSequence/ RemoteMap isn't public, and their __init__ methods(fixed by this PR) are called internally. On the other hand, it seems that current test code covers public API.
I wonder whether it's better to also cover private APIs(In this case, __init__ of RemoteSequence/ RemoteMap).

Best regards.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think tests of internal classes are needed, if both classes are already covered by public API, there is no need to add a test.

Copy link
Contributor Author

@FGtatsuro FGtatsuro Mar 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thank you or your comment!

@bfredl bfredl merged commit debcde0 into neovim:master Mar 17, 2018
@FGtatsuro FGtatsuro deleted the remove_deadcode branch March 18, 2018 03:08
bfredl added a commit that referenced this pull request Apr 29, 2018
This release adds support for Python 3.7 which now is in beta. `async`
is a keyword in python3.7, and it should be replaced with `async_` when
used as a keyword argument to API methods. `async` is still supported
with python 2.7 and 3.6 for the moment, but considered deprecated.

Also, integration with the in-process lua interpreter in Nvim is now supported.

Changes since 0.2.4:

* debcde0 clean up remote object implementation (#313)
* e880fe7 Guidelines for local plugin development (#317)
* 1ab98e8 Update the working directory on DirChanged for legacy plugins (#296)
* d53415d Fix SyntaxError in py3.7: use `async_` instead of `async` (#274)
* b65f62d Use `pytest` as `nosetests` are not longer maintained (#266)
* d9aed96 Support using lua functions. `buf.update_higlights` as an internal example (#325)
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.

3 participants