Skip to content

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Jun 19, 2017

See issue: #390

Remove DAB as it is hard to maintain / not much usage / DAB is still
experimental in Docker and there hasn't been much movement:
moby/moby#26876

MarkDeprecated does not work at the moment due to issue:
#652

However, that is not a blocker as we fatalF within ValidateFlags

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 19, 2017
@cdrage
Copy link
Member Author

cdrage commented Jun 19, 2017

dabbing

ping @sebgoa @kadel

@sebgoa
Copy link
Contributor

sebgoa commented Jun 19, 2017

so +1 , except that as you mention MarkDeprecated does not work.

@cdrage
Copy link
Member Author

cdrage commented Jun 19, 2017

@sebgoa Yes, an issue with Cobra, which already has a fix, see: spf13/cobra#466 will just have to wait until it's merged upstream. But since we error out anyways if --bundle is passed, this will still work. We'll just have to update vendoring once spf13/cobra#466 is merged 👍

@cdrage cdrage changed the title Remove dab Disable dab Jun 19, 2017
@cdrage
Copy link
Member Author

cdrage commented Jun 19, 2017

I've also updated the commit title / information.

We're disabling DAB not removing it, as the code for conversion is still there for future use!

See issue: kubernetes#390

Disable DAB as it is hard to maintain / not much usage / DAB is still
experimental in Docker and there hasn't been much movement:
moby/moby#26876

MarkDeprecated does not work at the moment due to issue:
kubernetes#652

However, that is not a blocker as we `fatalF` within `ValidateFlags`
@cdrage
Copy link
Member Author

cdrage commented Jun 20, 2017

Okay. Tests pass now. @kadel @surajssd want to do a quick review?

@surajnarwade
Copy link
Contributor

works for me :)

@procrypt
Copy link

procrypt commented Jun 21, 2017

@cdrage LGTM and +1 for disabling DAB 💃

@procrypt
Copy link

@cdrage Do we need to write the functional tests for dab in golang since we are disabling it.

@pradeepto
Copy link

@cdrage Do we need to write the functional tests for dab in golang since we are disabling it.

No, we definitely shouldn't but I will let @cdrage and others take the final call.

@cdrage
Copy link
Member Author

cdrage commented Jun 21, 2017

@procrypt @surajnarwade Remeber to go through the review process and hit "approve" 👍 But since I got two confirmations, let's go ahead and merge this!

@cdrage cdrage merged commit cf39f78 into kubernetes:master Jun 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants