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

Autoimport refactor #485

Closed
wants to merge 7 commits into from
Closed

Conversation

bagel897
Copy link
Contributor

@bagel897 bagel897 commented Jun 5, 2022

Description

This is a refactor of some parts of autoimport. It unifies cache generation between modules and resources and other methods.
I merged some changes from the CI branch regarding autoimport. I don't really intend to merge this branch because I intend to move autoimport to its own package/repository for a couple reasons:

  1. Since I'm responsible for it, I'd be able to maintain it myself
  2. It can be more easily integrated in projects like Jedi and pylsp without relying on the rest of rope.
  3. I can use more modern tooling/items more easily
  4. Its easier to maintain, since I'd be able to implement the entire rope API atop a much smaller subset
  5. Rope isn't really a completion library

I'd take one of the following approaches:

  1. Take this branch, move autoimport to its own repository, and later use that in rope, replacing the sqlite3 implementation while (at least mostly) preserving compatibility. Some of the refactors here make this easier. The other library would be kept as an optional dependency for the sqlite implementation.
  2. If that isn't an option, consider merging this PR (after add windows and mac CI #481, sorry for making it messy) and link the new version in documentation. I may remove rope-specific APIs in the new implementation.

@bagel897
Copy link
Contributor Author

bagel897 commented Jun 9, 2022

I'm working on the autoimport code here: https://github.com/bageljrkhanofemus/autoimport-core

I won't be doing as much for a while - at least a few days.
Since this PR isn't intended to be merged, I'll close it.

@lieryan
Copy link
Member

lieryan commented Nov 7, 2023

Since I'm responsible for it, I'd be able to maintain it myself

@bagel897 something I've been meaning to ask, do you want to have committer/co-maintainer role on rope if it will make it much easier for you to maintain autoimport (and you'd be welcome to contribute to other parts of rope if you wish)?

PS: Reach me out at [email protected] if you want to discuss this in private.

@bagel897
Copy link
Contributor Author

bagel897 commented Nov 7, 2023

@lieryan I'm not sure if I can commit the time for that atm, thanks for the offer.

@lieryan
Copy link
Member

lieryan commented Nov 8, 2023

Understood. No worries then. Thank you for considering.

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