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.

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

refactor: moved etcd initialiation to apisix/cli/etcd.lua #2685

Merged
merged 4 commits into from
Nov 24, 2020

Conversation

tokers
Copy link
Contributor

@tokers tokers commented Nov 10, 2020

Signed-off-by: Alex Zhang zchao1995@gmail.com

What this PR does / why we need it:

Move the etcd initialization into apisix/cli/etcd.lua, some auxiliary functions were moved to apisix/cli/etcd.lua and the yaml configuration file parsing function was moved to apisix/cli/file.lua.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM.

It is recommended to submit a small PR for easy review.

@membphis
Copy link
Member

haha, @tokers you need to rebase your branch

apisix/cli/etcd.lua Outdated Show resolved Hide resolved
@tokers
Copy link
Contributor Author

tokers commented Nov 18, 2020

@membphis Rebased.
@spacewander Changes about configuration vars was moved to apisix/cli/file.lua, please help me to check whether any faults brought in due to this rebasing.

apisix/cli/etcd.lua Outdated Show resolved Hide resolved
apisix/cli/file.lua Outdated Show resolved Hide resolved
apisix/cli/util.lua Show resolved Hide resolved
apisix/cli/etcd.lua Outdated Show resolved Hide resolved
apisix/cli/file.lua Outdated Show resolved Hide resolved
apisix/cli/file.lua Show resolved Hide resolved
apisix/cli/file.lua Show resolved Hide resolved
@tokers
Copy link
Contributor Author

tokers commented Nov 20, 2020

@membphis Need your review again since more changes are introduced. :)

@spacewander
Copy link
Member

@tokers
Need to solve the conflict.

@tokers
Copy link
Contributor Author

tokers commented Nov 24, 2020

@spacewander Resolved.

@spacewander spacewander merged commit 296f535 into apache:master Nov 24, 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.

5 participants