karmada icon indicating copy to clipboard operation
karmada copied to clipboard

improve karmadactl init

Open carlory opened this issue 2 years ago • 14 comments

What type of PR is this?

/kind feature

What this PR does / why we need it:

make the code more readable

Does this PR introduce a user-facing change?:

NONE

carlory avatar Jul 14 '22 14:07 carlory

/hold

waiting for #2191 to be merged

carlory avatar Jul 14 '22 14:07 carlory

/cc @lonelyCZ @prodanlabs

carlory avatar Jul 14 '22 14:07 carlory

waiting for #2191 to be merged

Thank @carlory , #2191 has been merged. /hold cancel

prodanlabs avatar Jul 14 '22 22:07 prodanlabs

/LGTM

prodanlabs avatar Jul 15 '22 01:07 prodanlabs

/cc @lonelyCZ @prodanlabs

carlory avatar Jul 16 '22 01:07 carlory

@prodanlabs Unfortunately, the e2e testing was failed.

the lgtm label is removed, could you review it again ?

carlory avatar Jul 17 '22 03:07 carlory

@lonelyCZ @prodanlabs updated

carlory avatar Jul 31 '22 12:07 carlory

I will review ASAP.

/assign

lonelyCZ avatar Aug 01 '22 07:08 lonelyCZ

Hi @carlory , please rebase this branch and resolve conflicts.

I just tested it that worked fine!

lonelyCZ avatar Aug 01 '22 08:08 lonelyCZ

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please ask for approval from lonelycz after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot avatar Aug 01 '22 08:08 karmada-bot

/lgtm

/assign @prodanlabs @RainbowMango

lonelyCZ avatar Aug 01 '22 09:08 lonelyCZ

/cc @RainbowMango

carlory avatar Aug 11 '22 15:08 carlory

Please take a look @RainbowMango @prodanlabs

carlory avatar Aug 16 '22 12:08 carlory

I can see this PR include two changes:

  1. rename package pkg/karmadactl/cmdinit/options to pkg/karmadactl/cmdinit/constants
  2. move all options to pkg/karmadactl/cmdinit/kubernetes/options.go

My question is these are the options for pkg/karmadactl/cmdinit, why put it under kubernetes?

RainbowMango avatar Aug 18 '22 07:08 RainbowMango