Skip to content

Support multiple indecies in TolerantIndexBuilder#2795

Closed
mamazu wants to merge 5 commits intophpactor:masterfrom
mamazu:phpactor-1816
Closed

Support multiple indecies in TolerantIndexBuilder#2795
mamazu wants to merge 5 commits intophpactor:masterfrom
mamazu:phpactor-1816

Conversation

@mamazu
Copy link
Contributor

@mamazu mamazu commented Dec 1, 2024

The indexer had a constructor argument that is the Index. This is suboptimal if we want to support multiple indices. So now it's an argument when trying to index a file (or list of files).

@mamazu mamazu marked this pull request as draft December 1, 2024 14:01
@dantleech
Copy link
Collaborator

dantleech commented Dec 2, 2024

Can you describe the architecture you envision for this feature? Also beware, as is evident from other PRs, I haven't really got the time or motivation to deep dive into complex topics with Phpactor.

@mamazu
Copy link
Contributor Author

mamazu commented Dec 3, 2024

Well my main goal is to reduce the dependency on the index as a constructor argument and move it to the methods that actually need it. Furthermore I'm trying to split up the IndexAgentBuilder as it currently is also very intertwined with the Index. But as for the steps needed to take, I'm not sure myself yet.

@mamazu mamazu changed the title Draft: Supporting multiple indices Support Multiple indecies in TolerantIndexBuilder Jan 4, 2025
@mamazu mamazu changed the title Support Multiple indecies in TolerantIndexBuilder Support multiple indecies in TolerantIndexBuilder Jan 4, 2025
@mamazu mamazu marked this pull request as ready for review January 4, 2025 17:31
@dantleech
Copy link
Collaborator

So I don't think this is necessarily the way to go, I need to know what this will look like before knowing how to refactor it.

@dantleech
Copy link
Collaborator

@mamazu
Copy link
Contributor Author

mamazu commented Jan 4, 2025

Looks like a good idea. In the end I was thinking of the same thing. But I wasn't sure where to best place to put the distinction which index to use was.

But I think the lower the better. So that the amount of classes having to actually deal with an index is as small as possible.

@mamazu mamazu closed this Jan 8, 2025
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