Burn YAML with fire (kind of) #5533
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
smarterclayton
force-pushed
the
burn_yaml_with_fire
branch
from
150a7e2 to
0123f1b
Compare
| } | ||
|
|
||
| // yamlCodec implements Codec | ||
| var _ Codec = yamlCodec{} |
There was a problem hiding this comment.
What's the point of this line?
There was a problem hiding this comment.
To prove that yamlCodec implements Codec. We started doing that in other places where there's a type that should implement a named interface.
----- Original Message -----
// CodecFor returns a Codec that invokes Encode with the provided version.
func CodecFor(scheme *Scheme, version string) Codec {
return &codecWrapper{scheme, version}
}+// yamlCodec converts YAML passed to the Decoder methods to JSON.
+type yamlCodec struct {
- // a Codec for JSON
- Codec
+}
+// yamlCodec implements Codec
+var _ Codec = yamlCodec{}What's the point of this line?
Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5533/files#r26594752
|
lgtm, pending e2e run (in progress). |
|
Great, thanks. FYI/FWIW, Kubelet already explicitly calls yaml.Unmarshal: |
|
AFAICT, we're still not setting Content-Type in the client, and we're not documenting the accepted Content-Type in apiserver: |
Remove last usage of ResourceFromFile
Enables clients to optionally handle YAML
Base codecs no longer automically handle YAML. Instead, clients must convert to JSON first via yaml.ToJSON and runtime.YAMLDecoder.
smarterclayton
force-pushed
the
burn_yaml_with_fire
branch
from
0123f1b to
6918a4d
Compare
|
rebased |
|
Did we get results of the e2e run? |
smarterclayton
added
area/apiserver
area/kubectl
kind/cleanup
|
Doing an e2e run now |
|
e2e passing this morning (locally). This lgtm to merge. |
j3ffml
added
the
lgtm
|
Merge away - don't wait for my tests (saves me a few hours) |
Burn YAML with fire (kind of)
|
This seems to have broken tests for pkg/kubelet/config |
|
Fixing that |
|
Crossed merges |
|
Also seems to have broken integration On Mon, Mar 23, 2015 at 11:40 AM, Clayton Coleman notifications@github.com
|
|
Integration is fixed in #5775 ----- Original Message -----
|
Remove automatic decoding of YAML via core Codec and replace with opt-in decoding.
Partially addresses #1004
Needs an e2e run in case there's a missed spot.