Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

*: export advertise-addr as the command arg; use master-addr #484

Merged
merged 2 commits into from
Feb 20, 2020
Merged

*: export advertise-addr as the command arg; use master-addr #484

merged 2 commits into from
Feb 20, 2020

Conversation

csuzhangxc
Copy link
Member

What problem does this PR solve?

export advertise-addr to make DM easier to use in cloud.

What is changed and how it works?

export advertise-addr as the command arg; use master-addr

Check List

Tests

  • Unit test
  • Integration test

@csuzhangxc csuzhangxc added priority/normal Minor change, requires approval from ≥1 primary reviewer status/PTAL This PR is ready for review. Add this label back after committing new changes type/feature New feature labels Feb 19, 2020
@csuzhangxc
Copy link
Member Author

@WangXiangUSTC @lichunzhu PTAL

lichunzhu
lichunzhu reviewed Feb 19, 2020
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

Should we add a UT for this?
Rest LGTM

dm/master/config.go Outdated Show resolved Hide resolved
@lichunzhu lichunzhu added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Feb 19, 2020
lichunzhu
lichunzhu reviewed Feb 19, 2020
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

LGTM

@lichunzhu
Copy link
Contributor

/run-all-tests tidb=release-3.0

WangXiangUSTC
WangXiangUSTC reviewed Feb 20, 2020
View reviewed changes
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC WangXiangUSTC added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Feb 20, 2020
@csuzhangxc csuzhangxc merged commit 8a2d2b0 into pingcap:ha-dev Feb 20, 2020
@csuzhangxc csuzhangxc deleted the advertise-addr branch February 20, 2020 03:40
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020
@csuzhangxc
*: export advertise-addr as the command arg; use master-addr (pin…
4e1de4c 
…gcap#484)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lichunzhu lichunzhu lichunzhu left review comments

@WangXiangUSTC WangXiangUSTC WangXiangUSTC left review comments

Assignees
No one assigned
Labels
priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/feature New feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@csuzhangxc @lichunzhu @WangXiangUSTC