Skip to content

New flyway start property: quarkus.flyway.clean-at-start #5697

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
Merged
merged 2 commits into from
Dec 6, 2019
Merged

New flyway start property: quarkus.flyway.clean-at-start #5697

merged 2 commits into from
Dec 6, 2019

Conversation

viniciusfcf
Copy link
Contributor

No description provided.

@viniciusfcf
Copy link
Contributor Author

viniciusfcf commented Nov 22, 2019 •
edited
Loading

Related to the issue #4232

@viniciusfcf viniciusfcf changed the title PR from quarkus master New flyway start property: quarkus.flyway.clean-at-start Nov 22, 2019
@gastaldi gastaldi added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Nov 22, 2019
@gastaldi
Copy link
Contributor

Looks like you have some unrelated additional commits in this PR. Can you rebase or cherry-pick just the bb8d861 commit? Thanks

gastaldi
gastaldi requested changes Nov 22, 2019
View reviewed changes
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Added some comments.

gsmet
gsmet requested changes Nov 22, 2019
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Let's wait for the multiple datasource support being merged before considering merging this one.

I have plans to review it early next week.

@viniciusfcf
Copy link
Contributor Author

viniciusfcf commented Nov 22, 2019 •
edited
Loading

Looks like you have some unrelated additional commits in this PR. Can you rebase or cherry-pick just the bb8d861 commit? Thanks

@gastaldi I think I fixed it.

@viniciusfcf viniciusfcf requested a review from gastaldi November 22, 2019 22:17
@gastaldi
Copy link
Contributor

Sorry, but it is still not rebased properly. In your branch, do git rebase master and then squash the commits to a single one (git rebase -i HEAD~2)

@gastaldi
Copy link
Contributor

gastaldi commented Nov 22, 2019 •
edited
Loading

Ah looks like your master branch is used in this PR. It's easier if you provide a PR from a separate branch

gastaldi
gastaldi reviewed Nov 23, 2019
View reviewed changes
integration-tests/flyway/src/test/java/io/quarkus/it/flyway/FlywayFunctionalityTest.java
public void testFlywayQuarkusFunctionality() {
when().get("/flyway/migrate").then().body(is("1.0.1"));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line break necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using Quarkus Code Style and before Commit, I run "mvn clean install" comands to format the code corretly

gastaldi
gastaldi reviewed Nov 23, 2019
View reviewed changes
integration-tests/flyway/src/main/java/io/quarkus/it/flyway/FlywayFunctionalityResource.java
"Version is null! Migration was not applied");
return version.toString();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line break necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using Quarkus Code Style and before Commit, I run "mvn clean install" comands to format the code corretly

gastaldi
gastaldi approved these changes Nov 23, 2019
View reviewed changes
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for being too picky. Good job!

@gastaldi gastaldi removed the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Nov 23, 2019
@gastaldi gastaldi added this to the 1.1.0 milestone Nov 23, 2019
@viniciusfcf
Copy link
Contributor Author

@gastaldi . Thank's your patience, it's my first PR in a open source project

@gsmet
Copy link
Member

gsmet commented Dec 6, 2019

I won't have the time to get the multiple datasource support in and this one looks valuable so I'm getting this one in.

@gsmet gsmet merged commit 4e9cc1e into quarkusio:master Dec 6, 2019
@gsmet
Copy link
Member

gsmet commented Dec 6, 2019

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gastaldi gastaldi gastaldi approved these changes

@gsmet gsmet gsmet approved these changes

Assignees

No one assigned

Labels

release/noteworthy-feature

Projects

None yet

Milestone

1.1.0.CR1

Development

Successfully merging this pull request may close these issues.

3 participants

@viniciusfcf @gastaldi @gsmet