Skip to content

Issue #1987: Migrate command convert-to-interleaved-storage#1988

Merged
sijie merged 7 commits intoapache:masterfrom
zymap:command-ctis
Mar 25, 2019
Merged

Issue #1987: Migrate command convert-to-interleaved-storage#1988
sijie merged 7 commits intoapache:masterfrom
zymap:command-ctis

Conversation

@zymap
Copy link
Member

@zymap zymap commented Mar 15, 2019

Descriptions of the changes in this PR:

  • Migrate command convert-to-interleaved-storage

Motivation

Changes

  • Add command in bookiegroup

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

overall looks good. left one minor comment.

@Test
public void testConvertToInterleavedStorageCommand() {
ConvertToInterleavedStorageCommand cmd = new ConvertToInterleavedStorageCommand();
Assert.assertTrue(cmd.apply(bkFlags, new String[] { "" }));
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to verify the command is executing as expected. e.g. verify if those methods are called

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, i fix it. PTAL

Copy link
Member Author

Choose a reason for hiding this comment

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

Am I fix right?

@zymap
Copy link
Member Author

zymap commented Mar 16, 2019

run integration tests

@sijie
Copy link
Member

sijie commented Mar 19, 2019

run integration tests

@sijie
Copy link
Member

sijie commented Mar 20, 2019

@zymap it seems that CI is not passing, can you check?

@zymap
Copy link
Member Author

zymap commented Mar 20, 2019

@sijie ok, i will check it later

@zymap
Copy link
Member Author

zymap commented Mar 20, 2019

run bookkeeper-server bookie tests

@sijie sijie added this to the 4.10.0 milestone Mar 25, 2019
@sijie sijie merged commit 4de5983 into apache:master Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants