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

Unify the integration tests of Scalar DB on X #172

Merged
merged 3 commits into from
Mar 11, 2021
Merged

Unify the integration tests of Scalar DB on X #172

merged 3 commits into from
Mar 11, 2021

Conversation

brfrn169
Copy link
Collaborator

@brfrn169 brfrn169 commented Mar 9, 2021

@brfrn169 brfrn169 requested review from feeblefakie and yito88 March 9, 2021 05:39
@brfrn169 brfrn169 self-assigned this Mar 9, 2021
feeblefakie
feeblefakie reviewed Mar 10, 2021
View reviewed changes
Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

Great refactoring!!!
Overall looking pretty good, but I noticed the import statements are moved probably by Intellij unexpectedly.

...-test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java Outdated
static final String ANY_ID_1 = "id1";
static final String ANY_ID_2 = "id2";
private final ConsensusCommitManager manager;
import static org.assertj.core.api.Assertions.assertThat;
Copy link
Contributor

Choose a reason for hiding this comment

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

These static import statements (and normal ones) are probably moved by Intellij.
I ran java -jar google-java-format-1.7-all-deps.jar with command-line, and they are moved back to the original location.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yes... Since I use the Google java format plugin, I thought I followed the Google style even the import order. But it looks like the import order is not handled by the plugin... I will fix the import order for the files that are changed in this PR.

@brfrn169 brfrn169 requested a review from feeblefakie March 10, 2021 08:07
feeblefakie
feeblefakie approved these changes Mar 11, 2021
View reviewed changes
Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@feeblefakie feeblefakie merged commit 1faeeff into master Mar 11, 2021
@feeblefakie feeblefakie deleted the unify-integration-tests branch March 11, 2021 01:00
feeblefakie pushed a commit that referenced this pull request Mar 16, 2021
@brfrn169 @feeblefakie
Unify the integration tests of Scalar DB on X (#172)
116d515
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feeblefakie feeblefakie feeblefakie approved these changes

@yito88 yito88 Awaiting requested review from yito88

Assignees

@brfrn169 brfrn169

Labels
improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brfrn169 @feeblefakie