Skip to content

Conversation

@fsouza
Copy link
Contributor

@fsouza fsouza commented May 9, 2020

Leverages jedi's support for refactoring to implement rename.

This requires upgrading jedi. I can split this PR in two if preferred (first upgrade jedi, then add rename). Nevermind I see that #781 exists, once that's merged I can rebase.

Note: Jedi refactoring requires Python 3.6+, so I added 3.6 to CI. I didn't remove Python 3.5, but I believe at this point it's probably safe to do so. Thoughts? Also let me know if you'd rather go all the way to Python 3.8.

Closes #530.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/python-language-server, @fsouza! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@fsouza
Copy link
Contributor Author

fsouza commented May 9, 2020

Oh I see that the upgrade to jedi 0.17.0 is being handled in #781. I can rebase my changes once that's merged.

fsouza added a commit to fsouza/dotfiles that referenced this pull request May 9, 2020
@ccordoba12
Copy link
Contributor

I didn't remove Python 3.5, but I believe at this point it's probably safe to do so.

I think so too. Please use 3.6 instead of 3.5 in our CIs.

@ccordoba12 ccordoba12 changed the title Support for renaming with jedi Add support for renaming with Jedi May 10, 2020
@ccordoba12 ccordoba12 added this to the 0.32.0 milestone May 10, 2020
Copy link
Contributor

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks for this @fsouza! I left some small comments for you (please try to address them asap, so we can include your work in our next version).

@fsouza
Copy link
Contributor Author

fsouza commented May 10, 2020

Hi @ccordoba12, thanks for the feedback. I'll be able to address it later today or tomorrow morning (EDT). No need to hold the release if that doesn't work for you.

Thank you! :D

@ccordoba12
Copy link
Contributor

I'll be able to address it later today or tomorrow morning (EDT). No need to hold the release if that doesn't work for you.

If you can do it between today and tomorrow is fine. We still have to review and merge PR #754, which we plan to do tomorrow.

@fsouza
Copy link
Contributor Author

fsouza commented May 11, 2020

@ccordoba12 I think I got it all. Thanks for all the feedback!

Copy link
Contributor

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @fsouza for addressing our review so quickly! I just left a minor suggestion, otherwise looks good.

Co-authored-by: Carlos Cordoba <[email protected]>
@ccordoba12
Copy link
Contributor

Don't worry about the failures in our tests. They are due to a new flake8 version.

Copy link
Contributor

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @fsouza!

@ccordoba12 ccordoba12 merged commit f25259f into palantir:develop May 11, 2020
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.

Make default renaming work with Jedi

4 participants