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

#1049 Replace "Collapse and Expand Results" Solr query with "Result Grouping" query. #1053

Merged
merged 5 commits into from
May 2, 2023

Conversation

syefimov
Copy link
Contributor

@syefimov syefimov commented Apr 6, 2023

Ticket #1049
Replace "Collapse and Expand Results" Solr query with "Result Grouping" query.

Solr cloud results "Collapse/Expand" bug apache#1049
storm-crawler-solr bug. Missing DeletionBolt bolt code. apache#1050
@jnioche
Copy link
Contributor

jnioche commented Apr 6, 2023

thanks for the PR @syefimov

I would have been better to have one PR per issue. For instance, the addition of the DeletionBolt could be done while discussing / refining the other PR.

You have ignored my comment about the license header, your commits are not signed, the title of the PR is not indicative of what it does and the formatting recommendation (simply call 'mvn git-code-format:format-code -Dgcf.globPattern=**/*') seem not to have been followed.

Would you mind having another try? Thanks!

@syefimov
Copy link
Contributor Author

syefimov commented Apr 7, 2023 via email

@syefimov syefimov changed the title Update SolrSpout.java #1049 Replace "Collapse and Expand Results" Solr query with "Result Grouping" query. Apr 8, 2023
@syefimov
Copy link
Contributor Author

syefimov commented Apr 8, 2023

I removed DeletionBolt.java, committed (signed) formatted SolrSpout.java. I should create a separate branch for this commit in my repository. Will do it in a right way for a next one.
Thank you.

@apache apache deleted a comment from sonatype-lift bot Apr 11, 2023
@apache apache deleted a comment from sonatype-lift bot Apr 11, 2023
@jnioche
Copy link
Contributor

jnioche commented Apr 11, 2023

looks way better, thanks @syefimov

I don't think the formatting has been applied. Could you please run
mvn git-code-format:format-code -Dgcf.globPattern=**/
and push?

Would have been nice to be able to reproduce the problem that this PR is fixing but this would require #851 first

@jnioche
Copy link
Contributor

jnioche commented Apr 27, 2023

Hi @syefimov, how can I help you make progress on this PR?

@syefimov
Copy link
Contributor Author

syefimov commented Apr 27, 2023 via email

@syefimov
Copy link
Contributor Author

I put changes to the grouping query code, run formatting script again and committed changes. Hope it will work this time.

@jnioche jnioche added this to the 2.9 milestone May 2, 2023
@jnioche jnioche merged commit aa1ec38 into apache:master May 2, 2023
4 checks passed
@jnioche
Copy link
Contributor

jnioche commented May 2, 2023

thanks a lot @syefimov, this is great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants