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

LUCENE-10236: Add change entry #456

Merged
merged 3 commits into from
Dec 20, 2021

Conversation

zacharymorn
Copy link
Contributor

No description provided.

@@ -57,7 +57,9 @@ Optimizations

Bug Fixes
---------------------
(No changes)

* LUCENE-10236: Update field-weight used in CombinedFieldQuery scoring calculation.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe say a bit more about what really changed than just "update", e.g. stop duplicating norms and term frequencies when scoring in CombinedFieldQuery?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry. I've updated it accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mikemccand , just want to check back on this to see if the updated entry looks good to you?

Copy link
Member

Choose a reason for hiding this comment

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

Oh yes looks great sorry for the slow response!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, thanks @mikemccand for the review and approval!

Copy link
Member

@mikemccand mikemccand left a comment

Choose a reason for hiding this comment

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

Thanks @zacharymorn

@mikemccand
Copy link
Member

Hmm actually this issue was fixed in 9.0.0, I think?

@zacharymorn
Copy link
Contributor Author

Hmm actually this issue was fixed in 9.0.0, I think?

This change entry is for 07ee3ba#diff-27a1984493254d0af3e9c50caee47443abbbdc79a87b4c348300bcb317a0a5b2, as I forgot to add it in that merge. I checked the commit immediately after mine ad911df on main and saw the change entry used 9.1.0, so I assumed I should use that as well?

On the other hand, branch_9_0 doesn't contain my fix commit as I haven't back-ported it yet.

Not sure if I may miss something?

@zacharymorn
Copy link
Contributor Author

Hmm actually this issue was fixed in 9.0.0, I think?

This change entry is for 07ee3ba#diff-27a1984493254d0af3e9c50caee47443abbbdc79a87b4c348300bcb317a0a5b2, as I forgot to add it in that merge. I checked the commit immediately after mine ad911df on main and saw the change entry used 9.1.0, so I assumed I should use that as well?

On the other hand, branch_9_0 doesn't contain my fix commit as I haven't back-ported it yet.

Not sure if I may miss something?

Hi @mikemccand , just want to check back on this and see if you have any further concern?

@mikemccand
Copy link
Member

Aha, perfect, looks great! +1 to push. Sorry for the delay @zacharymorn.

@zacharymorn
Copy link
Contributor Author

Aha, perfect, looks great! +1 to push. Sorry for the delay @zacharymorn.

No problem! Thanks for the review and approval!

@zacharymorn zacharymorn merged commit 92e5391 into apache:main Dec 20, 2021
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