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.

Sign up for GitHub

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

Jump to bottom

Winduprule 385 hibernate limit offset #368

Merged
merged 9 commits into from Mar 8, 2019
Merged

Winduprule 385 hibernate limit offset #368

merged 9 commits into from Mar 8, 2019

Conversation

jonathanvila
Copy link
Member

@jonathanvila jonathanvila commented Mar 7, 2019 •
edited by mrizzi

@jonathanvila jonathanvila requested a review from mrizzi March 7, 2019 16:10
@PhilipCattanach
Copy link
Contributor

Jonathan,
I have tried running this locally and the test failed.
I suspect you've built it and tested it in isolation, and another Hibernate test file has triggered the rule
https://github.com/windup/windup-rulesets/blob/master/rules-reviewed/eap7/eap6/tests/data/data-hibernate4/hibernate.cfg.xml#L9
So the expected count and actual count of incidents is different
Also I think the file that has triggered the rule, should not have done
org.hibernate.dialect.Oracle12cDialect
but Oracle12cDialect is not one of the required dialects

@jonathanvila
Copy link
Member Author

@PhilipCattanach the issue is that I didnt put a WIP in the PR as I still had to test after moving the rule to hibernate4 file.
Oracle12 Dialect , you are right. As there is also a test for the no-namespace property this can lead to an error.
I'm checking this currently.

@jonathanvila jonathanvila changed the title Winduprule 385 hibernate limit offset WIP: Winduprule 385 hibernate limit offset Mar 8, 2019
jonathan and others added 2 commits March 8, 2019 15:29
@jonathanvila jonathanvila changed the title WIP: Winduprule 385 hibernate limit offset Winduprule 385 hibernate limit offset Mar 8, 2019
@PhilipCattanach
Copy link
Contributor

Tested the PR successfully - All good

mrizzi
mrizzi approved these changes Mar 8, 2019
View reviewed changes
Copy link
Member

@mrizzi mrizzi left a comment

Choose a reason for hiding this comment

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

everything worked in local testing and central builds 👍

@mrizzi mrizzi merged commit ed1b43d into windup:master Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrizzi mrizzi mrizzi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@jonathanvila @PhilipCattanach @mrizzi