Skip to content

orchestrator: initial copy#6593

Merged
sougou merged 1 commit intovitessio:initial-orchestrator-importfrom
planetscale:initial-orchestrator-import
Aug 23, 2020
Merged

orchestrator: initial copy#6593
sougou merged 1 commit intovitessio:initial-orchestrator-importfrom
planetscale:initial-orchestrator-import

Conversation

@shlomi-noach
Copy link
Copy Markdown
Contributor

@shlomi-noach shlomi-noach commented Aug 19, 2020 •
edited by deepthi
Loading

Context: #6582

This PR pulls changes into initial-orchestrator-import branch, not master, with initial orchestrator code import, so that the rest of #6582 can be refactored to merge into initial-orchestrator-import. This way #6582 will be more review-able.

Since we're not merging this branch into master, we can generally avoid the review process for this PR.

Copy from @sougou 's original commit:

  • go modules enabled
  • go -> go/vt/orchestrator
  • go/cmd -> go/cmd
  • vendor-> external: golib, raft, zk
  • imports fixed
  • lint checks are failing

DO NOT MERGE

@sougou @shlomi-noach
orc: initial copy
fabb237 
* go modules enabled
* go -> go/vt/orchestrator
* go/cmd -> go/cmd
* vendor-> external: golib, raft, zk
* imports fixed
* lint checks are failing

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@shlomi-noach shlomi-noach requested a review from sougou as a code owner August 19, 2020 05:21
@shlomi-noach shlomi-noach mentioned this pull request Aug 19, 2020
Merged
@sougou
Copy link
Copy Markdown
Contributor

sougou commented Aug 20, 2020

I assume the goal is to review the two PRs independently, but merge #6582 afterwards?

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

shlomi-noach commented Aug 20, 2020

The goal is to be able to review the "real" changes in #6582, other than the import itself of all of orchestrator's code.

@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Aug 21, 2020

The goal is to be able to review the "real" changes in #6582, other than the import itself of all of orchestrator's code.

I like this idea. What do we do about the failing linter checks?

@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Aug 21, 2020 •
edited
Loading

I guess we could rebase #6582 onto this branch to see the diffs. After review, we can rebase it back onto master and then merge.
Other option is to fix all the linter warnings in this PR, merge it, then rebase/merge #6582 and then merge that.

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Aug 21, 2020

My understanding is that we don't merge this PR, but use the other PR created against this branch to review the golint fixes?

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

shlomi-noach commented Aug 23, 2020

My understanding is that we don't merge this PR, but use the other PR created against this branch to review the golint fixes?

this branch does not exist yet, in vitessio/vitess -- this PR comes from planetscale/vitess. So my intention was to merge this PR -- this merges not into master but into orchestrator-import -- so really this should be a no brainer with no risk. Then, we can point the other PR against orchestrator-import branch. We can't do that until this is merged.

@sougou sougou merged commit 9fcc2bd into vitessio:initial-orchestrator-import Aug 23, 2020
@sougou
Copy link
Copy Markdown
Contributor

sougou commented Aug 23, 2020

So... This did not go as planned, because I had rebased ss-oc1 against master...
What I've now done is create another branch in planetscale (oc0), and created a PR from oc1 to oc0: planetscale#68
I think it amounts to the same thing

@shlomi-noach shlomi-noach deleted the initial-orchestrator-import branch August 23, 2020 07:19
@shlomi-noach
Copy link
Copy Markdown
Contributor Author

shlomi-noach commented Aug 23, 2020

ok cool. planetscale#68 looks good.

@askdba askdba added this to the v8.0 milestone Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sougou sougou Awaiting requested review from sougou

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v8.0

Development

Successfully merging this pull request may close these issues.

4 participants

@shlomi-noach @sougou @deepthi @askdba