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 ILuceneSearchQueryService from OrchardCore.Search.Lucene #12624

Merged
merged 5 commits into from
Oct 12, 2022

Conversation

Skrypt
Copy link
Contributor

@Skrypt Skrypt commented Oct 11, 2022

Remove it as requested.
It was doing no harm at all, to be honest.

@sebastienros I don't think it has to do with the renaming of the assembly either. It was decided and voted that we rename it and I've never said I would not rename it back if we were agreeing on that as a group. If there are issues with the Migration, as I said, I will fix them. But so far the issues are not about the feature itself but about Deployment plans which I fixed this week. This ILuceneSearchQueryService, I decided to not move it at all in the end because I wanted to keep as little change as possible in the OrchardCore.Search.Lucene module for people to migrate easily. It would have caused no issue at all if we would have kept it where it was in the OrchardCore.Search.Lucene module too but my mistake was to not remove the ISearchQueryService that I had copied in the OrchardCore.Search.Lucene.Abstraction module. Now, we did completely the opposite and I'm not saying it is wrong. It is what it should be.

@MikeAlhayek We merged in the Elasticsearch PR in the main branch because it needs to be tested by people. Sorry if it caused you issues but you can also wait for the releases to ensure it is stable first. This is what the main branch is for.

I will assess the other request in a different PR to remove the testing of Elasticsearch.

@Skrypt Skrypt added this to the 1.5 milestone Oct 11, 2022
@Skrypt
Copy link
Contributor Author

Skrypt commented Oct 11, 2022

@MikeAlhayek Please share any details for documentation that could be added about migrating from OrchardCore.Lucene to OrchardCore.Search.Lucene. We should add that now the OrchardCore.Search module takes in consideration the fact that we can have multiple search providers. Again, if someone was using the ISearchQueryService. It is no longer existing in 1.5. I was renamed to ILuceneSearchQueryService and IElasticSearchQueryService.

@MikeAlhayek
Copy link
Member

MikeAlhayek commented Oct 11, 2022

@MikeAlhayek Please share any details for documentation that could be added about migrating from OrchardCore.Lucene to OrchardCore.Search.Lucene. We should add that now the OrchardCore.Search module takes in consideration the fact that we can have multiple search providers. Again, if someone was using the ISearchQueryService. It is no longer existing in 1.5. I was renamed to ILuceneSearchQueryService and IElasticSearchQueryService.

Based on what I saw,

  • Remove the reference to OrchardCore.Lucene otherwise you'll have weird run time errors.
  • If the ISearchQueryService was used in any custom modules, change that to ILuceneSearchQueryService instead, unless you are using ElasticSearch then use IElasticSearchQueryService.

src/docs/releases/1.5.0.md Outdated Show resolved Hide resolved
@Skrypt Skrypt merged commit b129d20 into main Oct 12, 2022
@Skrypt Skrypt deleted the skrypt/remove-ilucenesearchqueryservice branch October 12, 2022 20:14
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