syscall/js: document ValueOf() panic

[markus-wa]

ValueOf() panics if x is not one of the expected types.

[gopherbot]

This PR (HEAD: 34a88ce) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/150138 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Paul Jolly:

Patch Set 1:

Richard - what pattern are we following here by panic-ing?

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Richard Musiol:

Patch Set 1:

Patch Set 1:

Richard - what pattern are we following here by panic-ing?

What do you mean by "what pattern"? It is similar to the reflect package which panics if you call a method on an unsupported type.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Paul Jolly:

Patch Set 1:

Richard - what pattern are we following here by panic-ing?

What do you mean by "what pattern"? It is similar to the reflect package which panics if you call a method on an unsupported type.

Thanks, exactly that. What example/pattern/approach are we using: answer, the reflect package.

So I think we should be looking to mirror what the reflect docs do/don't do here, would you agree?

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Markus:

Patch Set 1:

I would argue reflect should be documented accordingly as well in that case.

Many functions there already are, e.g.: https://golang.org/src/reflect/value.go?s=23604:23628#L724

At first glance it was a bit confusing because so many functions have it in the doc so I wasn't sure if it ValueOf would do some black magic to convert structs or if it would panic or if it would return some default value.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Paul Jolly:

Patch Set 1:

I would argue reflect should be documented accordingly as well in that case.

I'm not arguing against what you've proposed, I'm just commenting for clarity's sake that that's what we're trying to do here, namely follow the example of reflect.

Many functions there already are, e.g.: https://golang.org/src/reflect/value.go?s=23604:23628#L724

At first glance it was a bit confusing because so many functions have it in the doc so I wasn't sure if it ValueOf would do some black magic to convert structs or if it would panic or if it would return some default value.

Quite agree; if there is a good pattern exhibited by some functions in reflect, then a) it would make sense (all else being equal) to bring consistency to them all (in another CL) and b) follow the good pattern here in this CL.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Markus:

Patch Set 1:

Quite agree; if there is a good pattern exhibited by some functions in reflect, then a) it would make sense (all else being equal) to bring consistency to them all (in another CL) and b) follow the good pattern here in this CL.

Ah ok. Sounds good, thanks for clarifying :)

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Richard Musiol:

Patch Set 1: Code-Review+2

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/150138.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR is being closed because golang.org/cl/150138 has been merged.