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

Adding support to parameters referenceLiquibaseCatalogName and referenceLiquibaseSchemaName. #4352

Merged

Conversation

filipelautert
Copy link
Collaborator

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Adding support to parameters referenceLiquibaseCatalogName and referenceLiquibaseSchemaName.

Instead of adding a global parameter as referenceCatalogName and referenceSchemaName, it was added as a parameter only to the commands that would use it, namely: Diff, diffChangelog and SnapshotReference .

@filipelautert
Copy link
Collaborator Author

Hello @TymofiiKritsak - I implemented the requested changes and did some manual test to confirm that they are working. Do you plan to create any integration tests for them?

@filipelautert filipelautert marked this pull request as ready for review June 13, 2023 16:49
@filipelautert filipelautert removed the request for review from XDelphiGrl June 29, 2023 19:21
@rberezen
Copy link
Contributor

Hello @TymofiiKritsak - I implemented the requested changes and did some manual test to confirm that they are working. Do you plan to create any integration tests for them?

can we add unit tests here? I mean these parameters just build connection string, right? so maybe it's enough just unit tests?
cc @TymofiiKritsak

@rberezen
Copy link
Contributor

PD ticket created: https://datical.atlassian.net/browse/PD-3557

@TymofiiKritsak
Copy link
Contributor

Hi @filipelautert!
@rberezen has created a ticket to add function tests: https://datical.atlassian.net/browse/DAT-15325
Thank you both!

@rberezen
Copy link
Contributor

rberezen commented Jul 7, 2023

@filipelautert also agreed to add an integration h2 test for this one

@filipelautert
Copy link
Collaborator Author

@MalloD12 are you able to add the tests as discussed here?

@rberezen rberezen added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Jul 26, 2023
@rberezen
Copy link
Contributor

integration tests added, test-only ticket for fucntional tests created, all tests passed

@filipelautert filipelautert force-pushed the 4058-support-reference-liquibase-catalog-and-schema-name branch from ab8540e to b0b32cd Compare August 29, 2023 01:27
@filipelautert filipelautert merged commit d233ed0 into master Aug 30, 2023
28 of 29 checks passed
@filipelautert filipelautert deleted the 4058-support-reference-liquibase-catalog-and-schema-name branch August 30, 2023 12:00
@filipelautert filipelautert added this to the 1NEXT milestone Aug 30, 2023
@aprognimak
Copy link

Might have worked at some point but apparently some overzealous commandline validator doesn't think so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions sprint2023-52 TypeEnhancement
Projects
Archived in project
5 participants