Skip to content

HDDS-5707. List container supports replication factor filter.#2606

Merged
captainzmc merged 6 commits intoapache:masterfrom
ChenSammi:HDDS-5707
Oct 27, 2021
Merged

HDDS-5707. List container supports replication factor filter.#2606
captainzmc merged 6 commits intoapache:masterfrom
ChenSammi:HDDS-5707

Conversation

@ChenSammi
Copy link
Contributor

Copy link
Contributor

@JacksonYao287 JacksonYao287 left a comment

Choose a reason for hiding this comment

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

LGTM +1, thanks for the work @ChenSammi

Comment on lines 411 to 427
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can refactor the codes as below, i think it will make the logic more clear and efficient!

if (state == null){
    if(factor == null){
        .............
    } else{
         ............
    }
}else{
    if(factor == null){
       ...........
    } else{
          .......
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @JacksonYao287 for the code review. I will update a new commit later to addres the comments.

Copy link
Member

@captainzmc captainzmc left a comment

Choose a reason for hiding this comment

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

+1 the change looks good.

@captainzmc captainzmc merged commit f133303 into apache:master Oct 27, 2021
@captainzmc
Copy link
Member

Merged this, thanks @ChenSammi ‘ patch, and thanks @JacksonYao287 for the review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants