Skip to content

Conversation

@AlvinMooreSr
Copy link
Contributor

Added support for specifying the FDB Cluster IP:Port combination
Added support for specifying the number of port from which to choose
Swtiched tabs for spaces to make Markus happy 😃

sfc-gh-kmakino and others added 2 commits September 23, 2020 16:14
Added support fo specify the number of ports from which to choose
Swapped tabs for spaces to make Marcus happy 😃
Copy link
Collaborator

@sfc-gh-mpilman sfc-gh-mpilman left a comment

Choose a reason for hiding this comment

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

lgtm - I assume the functions themselves you didn't change semantically but just reformatted?

@AlvinMooreSr
Copy link
Contributor Author

@mpilman Functions are basically the same. I did cherry-picked the commit from previous PR so that we can apply same changes to BindingTester on release branch.

Copy link
Collaborator

@sfc-gh-kmakino sfc-gh-kmakino left a comment

Choose a reason for hiding this comment

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

LGTM


else
"${BINDIR}/fdbserver" --knob_disable_posix_kernel_aio=1 -C "${FDBCONF}" -p "${FDBCLUSTERTEXT}" -L "${LOGDIR}" -d "${WORKDIR}/fdb/${$}" &> "${LOGDIR}/fdbserver.log" &
if [ "${?}" -ne 0 ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was a bit confusing to me, as I initially thought we would be checking the return code from fdbserver. After talking to @AlvinMooreSr, I realized that this is to check whether the process was successfully sent to the background. It would be nice if we had a one-line comment here.

@sfc-gh-mpilman sfc-gh-mpilman merged commit bd3ca9c into apple:release-6.3 Sep 30, 2020
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