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

Improve deferrable validation for AddForeignKey constraints generator #567

Merged
merged 7 commits into from
Oct 30, 2023
Merged

Conversation

alero
Copy link
Contributor

@alero alero commented Apr 22, 2016

This is an effort to minimize the clutter in the config and the support hsqldb in junit developer testing with the same changesets that are using in Postgre in production/integration testing

- We use this feature in JUnit testing to enable usage of HSQLDB in tests (using Postgres in prod). To not enforce rewriting of the entire liquibase config a flag was added that can be set to change deferrable handling
@datical-jenkins datical-jenkins changed the title Support HSQL and Postgrees using the same Liquibase config LB-94 ⁃ Support HSQL and Postgrees using the same Liquibase config Mar 4, 2020
@datical-jenkins datical-jenkins changed the title LB-94 ⁃ Support HSQL and Postgrees using the same Liquibase config Support HSQL and Postgrees using the same Liquibase config Mar 5, 2020
@ro-rah
Copy link

ro-rah commented May 13, 2020

Hi @alero ,

Would you resolve branch conflict and consider adding tests for this PR? Here's some guidance on that below:

Thanks for your pull request!

Here’s what happens next:

A member of the Liquibase team will take a look at your contribution and may suggest:

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

# Conflicts:
#	liquibase-core/src/main/java/liquibase/database/core/MockDatabase.java
@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #567 into master will decrease coverage by 0.01%.
The diff coverage is 42.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #567      +/-   ##
============================================
- Coverage     47.72%   47.71%   -0.02%     
- Complexity     7474     7476       +2     
============================================
  Files           757      757              
  Lines         36274    36283       +9     
  Branches       6625     6627       +2     
============================================
- Hits          17312    17311       -1     
- Misses        16659    16663       +4     
- Partials       2303     2309       +6     
Impacted Files Coverage Δ Complexity Δ
...ore/src/main/java/liquibase/database/Database.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...ain/java/liquibase/database/core/MockDatabase.java 35.22% <0.00%> (-0.21%) 50.00 <0.00> (ø)
...ase/change/core/AddForeignKeyConstraintChange.java 71.42% <20.00%> (-2.30%) 50.00 <2.00> (+2.00) ⬇️
...nerator/core/AddForeignKeyConstraintGenerator.java 60.71% <57.14%> (-2.75%) 12.00 <0.00> (ø)
.../java/liquibase/database/AbstractJdbcDatabase.java 58.04% <100.00%> (+0.07%) 213.00 <1.00> (+1.00)
...re/src/main/java/liquibase/util/ISODateFormat.java 77.08% <0.00%> (-2.09%) 15.00% <0.00%> (-1.00%)
...e/src/main/java/liquibase/util/DependencyUtil.java 91.22% <0.00%> (-0.88%) 0.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd55e14...91eeebc. Read the comment docs.

@ro-rah
Copy link

ro-rah commented May 13, 2020

@alero thanks for getting rid of the merge conflict! I'll size it as a medium, but happy to size it low if we get tests added.

@filipelautert
Copy link
Collaborator

Hello @alero ! A lot changed in the last years on Liquibase. Is this change still required?

@alero
Copy link
Contributor Author

alero commented Jul 31, 2023

Hi @filipelautert we still use this adaption and have actually forked it last month again to be up to date with latest releases

@filipelautert
Copy link
Collaborator

@alero ! Thanks for the quick answer. Do you think you can fix the conflicts (as we renamed liquibase-core to liquibase-standard and so on) so I can keep on moving this one along?

@filipelautert filipelautert self-requested a review October 23, 2023 14:24
@filipelautert
Copy link
Collaborator

Hello @alero ! I fixed the conflicts and tried to push the fix to your master branch but I don't have permission to do so. Would you be able to grant permission or fix the conflicts ?

@alero
Copy link
Contributor Author

alero commented Oct 23, 2023

Hello @filipelautert I have added you to the repo, so should be able to push now and thank you for the support.

@filipelautert
Copy link
Collaborator

Hello @filipelautert I have added you to the repo, so should be able to push now and thank you for the support.

Thanks , almost there! But seems the branch is protected and I can't push to your master anyway, so I created PR https://github.com/alero/liquibase/pull/1 in your repo to your master 🤣 could you approve and merge it?

Merge cnflicts on Liquibase Pr 567
@alero
Copy link
Contributor Author

alero commented Oct 23, 2023

approved and merged it to master

Copy link
Collaborator

@filipelautert filipelautert left a comment

Choose a reason for hiding this comment

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

7 years later it's still alive!

@sonarcloud
Copy link

sonarcloud bot commented Oct 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

77.4% 77.4% Coverage
0.0% 0.0% Duplication

@filipelautert filipelautert changed the title Support HSQL and Postgrees using the same Liquibase config Improve deferrable validation for AddForeignKey constraints generator Oct 24, 2023
@filipelautert filipelautert merged commit 7aea5d9 into liquibase:master Oct 30, 2023
36 of 43 checks passed
@filipelautert filipelautert added this to the 1NEXT milestone Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants