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

[WINDUPRULE-900] Create ruleset for hibernate search 5 to 6 migration #839

Merged
merged 9 commits into from
Jan 20, 2023

Conversation

jmle
Copy link
Contributor

@jmle jmle commented Jan 10, 2023

See Jira ticket.
All rules extracted from the migration guide.

@PhilipCattanach
Copy link
Contributor

PhilipCattanach commented Jan 11, 2023

@jmle - First off let me say thanks you for the huge amount of work that you have put into this ruleset.
I checked all of the 6.0 Migration Guide off against the rules and didn't find any gaps. Very well done.

The first point in the name of the rules file. It is misspelled hibiernate-search instead of hibernate-search and the corresponding test.windup.xml file should have the rulePath element pointing back to the hibernate-search.windup.xml file.
My next point is about the target. Shouldn't the eap targetTechnology be versionRange="[8,)"

However I also think that we should have targetTechnology id="hibernate" versionRange="[6,)"

I think we also need to do is have another ruleset for the issues in https://docs.jboss.org/hibernate/search/6.1/migration/html_single/ ?

The ruleset to cover the 6.1 rules should have 2 targets as well
targetTechnology id="eap" versionRange="[8,)"
targetTechnology id="hibernate" versionRange="[6.1,)"

I'll continue with my testing and let you know if I spot anything else.
Thanks again.

@PhilipCattanach
Copy link
Contributor

@jmle - I have submitted a PR for the (6.0) issues in the comment above. Thanks.

Copy link
Contributor

@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.

That's awesome, thanks a lot for working on this.

I'm not sure I spotted everything (this is a lot of work!) but I did notice what looks like a mistake.

@yrodiere
Copy link
Contributor

However I also think that we should have targetTechnology id="hibernate" versionRange="[6,)"

Note that Hibernate Search 6.0 is only compatible with Hibernate ORM 5.4 to 5.6. See https://hibernate.org/search/releases/6.0/#compatibility

Hibernate Search 6.1+ is still compatible with Hibernate ORM 5.6 by default, but can also work with Hibernate ORM 6.0+ through different Maven artifacts; see https://docs.jboss.org/hibernate/search/6.2/reference/en-US/html_single/#other-integrations-orm6
Those -orm6 artifacts are the only ones shipped with EAP 8.

…-900

WINDUPRULE-900 Hibernate Search 6.0 Rules minor corrections from PR R…
@jmle
Copy link
Contributor Author

jmle commented Jan 16, 2023

However I also think that we should have targetTechnology id="hibernate" versionRange="[6,)"

Note that Hibernate Search 6.0 is only compatible with Hibernate ORM 5.4 to 5.6. See https://hibernate.org/search/releases/6.0/#compatibility

Hibernate Search 6.1+ is still compatible with Hibernate ORM 5.6 by default, but can also work with Hibernate ORM 6.0+ through different Maven artifacts; see https://docs.jboss.org/hibernate/search/6.2/reference/en-US/html_single/#other-integrations-orm6 Those -orm6 artifacts are the only ones shipped with EAP 8.

@PhilipCattanach I just merged your PR and afterwards I saw this. We might need to make an additional rule/ruleset to cater for the Hibernate ORM dependency versions when migrating, if I understood correctly

@jmle jmle requested a review from yrodiere January 18, 2023 11:16
Copy link
Contributor

@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.

As requested, I reviewed this in full.

I think I spotted everything there was to spot, but it's a large PR :)

Details below.

@jmle jmle merged commit d222e1c into windup:master Jan 20, 2023
@mrizzi mrizzi added auto-backport Set the PR for being backported auto-backport-to-releases/6.1.z Backport this issue to releases/6.1.z branch labels Feb 23, 2023
github-actions bot pushed a commit that referenced this pull request Feb 23, 2023
…#839)

* [WINDUPRULE-900] Create ruleset for hibernate search 5 to 6 migration

* [WINDUPRULE-900] Small fixes

* [WINDUPRULE-900] Add jar dependency

* WINDUPRULE-900 Hibernate Search 6.0 Rules minor corrections from PR Review

* [WINDUPRULE-900] Fix rule

* Update rules/rules-reviewed/hibernate/hibernate-search.windup.xml

Co-authored-by: Yoann Rodière <yoann@hibernate.org>

* Update rules/rules-reviewed/hibernate/hibernate-search.windup.xml

Co-authored-by: Yoann Rodière <yoann@hibernate.org>

* [WINDUPRULE-900] Add link for dependencies rule

Co-authored-by: PhilipCattanach <pcattana@redhat.com>
Co-authored-by: Yoann Rodière <yoann@hibernate.org>
(cherry picked from commit d222e1c)
@github-actions
Copy link

💚 All backports created successfully

Status Branch Result
releases/6.1.z

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

mrizzi pushed a commit that referenced this pull request Feb 23, 2023
…#839) (#875)

* [WINDUPRULE-900] Create ruleset for hibernate search 5 to 6 migration

* [WINDUPRULE-900] Small fixes

* [WINDUPRULE-900] Add jar dependency

* WINDUPRULE-900 Hibernate Search 6.0 Rules minor corrections from PR Review

* [WINDUPRULE-900] Fix rule

* Update rules/rules-reviewed/hibernate/hibernate-search.windup.xml

Co-authored-by: Yoann Rodière <yoann@hibernate.org>

* Update rules/rules-reviewed/hibernate/hibernate-search.windup.xml

Co-authored-by: Yoann Rodière <yoann@hibernate.org>

* [WINDUPRULE-900] Add link for dependencies rule

Co-authored-by: PhilipCattanach <pcattana@redhat.com>
Co-authored-by: Yoann Rodière <yoann@hibernate.org>
(cherry picked from commit d222e1c)

Co-authored-by: Juan Manuel Leflet Estrada <jleflete@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Set the PR for being backported auto-backport-to-releases/6.1.z Backport this issue to releases/6.1.z branch
Projects
None yet
4 participants