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

HHH-19284 : Refactor - Extract duplicated vector distance function registration #9904

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

0xC0FFE2
Copy link

Extracted duplicated vector distance function registration code into a helper method to improve code clarity and maintenance.

https://hibernate.atlassian.net/browse/HHH-19284


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


@0xC0FFE2 0xC0FFE2 marked this pull request as draft March 26, 2025 11:16
@0xC0FFE2 0xC0FFE2 marked this pull request as ready for review March 26, 2025 11:39
@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Mar 26, 2025

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [f3cbdd4]

› This message was automatically generated.

@0xC0FFE2 0xC0FFE2 changed the title HHH-19284 : Extract duplicated vector distance function registration HHH-19284 : Refactor - Extract duplicated vector distance function registration Mar 27, 2025
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Hello @0xC0FFE2 ,

Thanks for your contribution.

While I agree your code is a slight improvement, I want to stress that refactoring for the sake of it is not something the whole Hibernate team sees with a good eye, especially if it's just some very localized problem: the cost of doing that refactoring and reviewing it often outweighs the benefits.

I'm ready to merge this, but:

  1. If you are looking for ways to contribute to Hibernate, please have a look at the good first issues.
  2. Please rebase your branch on the main branch to remove the "merge commit" -- we don't allow those.

@gavinking
Copy link
Member

I'm ready to merge this

I for one would not merge this, not unless a similar change is done in the VectorContributors for the other dialects. As it is, this introduces an inconsistency between the dialects.

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