Made JSON encoder a public function #443
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
lib/json/json.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| // Encodes the given Starlark value as a string in JSON format. |
There was a problem hiding this comment.
// Encode returns the JSON encoding of a Starlark value.
lib/json/json.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("%s: %v", b.Name(), err) | ||
| } | ||
| return starlark.String(s), nil | ||
| return starlark.String(string(bytes[:])), nil |
|
Two questions: First, I wasn't sure if Decode should also accept []byte, to have some symmetry with Encode. Second, I wasn't sure if I should add some tests. I started to add some, but realized I was testing basically everything that's already being tested. |
Yes, I think it should.
You could add one test to ensure that a value can be encoded and then decoded to ensure there are no gross errors, but I wouldn't bother testing all the cases of JSON. |
This one is pretty simple. I found myself starting to write a bunch of code to encode a Starlark value as JSON, when I realized that oh hey, there's a JSON encoder already in the project. It's just hidden.