Last Week
shurcooL commented on Using HTML5 history gives a nil ref… in github.com/dominikh/go-js-dom1d
This is because `window.History()` is not implemented, see #41.
This is because `window.History()` is not implemented, see #41.
shurcooL commented on document standard way to recognize … in github.com/golang/go2d> in #13560 (comment) bradfitz said the wiki would be edited but I can't seem to find anything on the wiki
He said:
> The wiki seems like it would just be edited by somebody without discussion.…
> in #13560 (comment) bradfitz said the wiki would be edited but I can't seem to find anything on the wiki
He said:
> The wiki seems like it would just be edited by somebody without discussion.…shurcooL commented on Robust Header Check in github.com/bradleyfalzon/ghinstallation2dI still see 3 lines of comments in `transport.go` file that don't have a space after `//`, but that's a minor thing. I don't have any other comments, I think this is just waiting on a review from @br…
I still see 3 lines of comments in `transport.go` file that don't have a space after `//`, but that's a minor thing. I don't have any other comments, I think this is just waiting on a review from @br…shurcooL commented on Suggestion: Allow `[]float32` or ot… in github.com/neelance/go3dFor posterity, it's issue golang/go#25532.
For posterity, it's issue golang/go#25532.shurcooL commented on runtime: implement timers, JavaScri… in github.com/golang/go3dAs far as I know, all modern browsers support the basic Fetch API by now, but not all of them support streams, which are required for streaming the response body. E.g., see https://caniuse.com/#searc…
As far as I know, all modern browsers support the basic Fetch API by now, but not all of them support streams, which are required for streaming the response body. E.g., see https://caniuse.com/#searc…shurcooL commented on runtime: implement timers, JavaScri… in github.com/golang/go3dI'm happy to offer help with implementing and/or reviewing here as well.
I'm the author of the original Fetch API-based `http.RoundTripper` implementation [in GopherJS](https://github.com/gopherjs…
I'm happy to offer help with implementing and/or reviewing here as well.
I'm the author of the original Fetch API-based `http.RoundTripper` implementation [in GopherJS](https://github.com/gopherjs…shurcooL commented on Support Preview hovercard API (#87… in github.com/google/go-github4dIt would be good to place `Hovercard` on top of `UserContext`. The general pattern is higher level identifiers should come first. They are easier to understand, and they set context for the lower lev…
It would be good to place `Hovercard` on top of `UserContext`. The general pattern is higher level identifiers should come first. They are easier to understand, and they set context for the lower lev…shurcooL commented on cannot convert math.MaxInt64 in github.com/gopherjs/gopherjs5d> `int(math.MaxInt64)` will never work, anywhere
Not sure what you mean, but it would work on a 64-bit environment where `int` is 64 bits.
> `int(math.MaxInt64)` will never work, anywhere
Not sure what you mean, but it would work on a 64-bit environment where `int` is 64 bits.shurcooL commented on cannot convert math.MaxInt64 in github.com/gopherjs/gopherjs5dGopherJS emulates a 32-bit environment. `int` is 32 bits, so the `math.MaxInt64` constant is too large for it. It will fit into a `int64` variable though.
See https://github.com/gopherjs/gopherjs#ge…
GopherJS emulates a 32-bit environment. `int` is 32 bits, so the `math.MaxInt64` constant is too large for it. It will fit into a `int64` variable though.
See https://github.com/gopherjs/gopherjs#ge…shurcooL commented on Cannot go get install in github.com/gopherjs/gopherjs5dIt looks like your `golang.org/x/tools` history has diverged from upstream for some reason. Either you made some local changes to it, or the upstream has force-pushed an incompatible commit (less lik…
It looks like your `golang.org/x/tools` history has diverged from upstream for some reason. Either you made some local changes to it, or the upstream has force-pushed an incompatible commit (less lik…Earlier
shurcooL commented on Proposal: Official GopherJS Docker … in github.com/gopherjs/gopherjs1wI personally don't need this myself at this time. But if you want to do it (for yourself and for others), I don't see why you shouldn't.
It might be a good idea to start this in a place where it's e…
I personally don't need this myself at this time. But if you want to do it (for yourself and for others), I don't see why you shouldn't.
It might be a good idea to start this in a place where it's e…shurcooL commented on js.ValueOf(0).Length() returns -429… in github.com/neelance/go1wFor precedent, https://godoc.org/reflect#Value.Len says:
> Len returns v's length. It panics if v's Kind is not Array, Chan, Map, Slice, or String.
Panicking (and documenting the panic) seems most …
For precedent, https://godoc.org/reflect#Value.Len says:
> Len returns v's length. It panics if v's Kind is not Array, Chan, Map, Slice, or String.
Panicking (and documenting the panic) seems most …shurcooL commented on Avoid unused method receiver names in dmitri.shuralyov.com/idiomatic-go1w
@ilius, not to my knowledge. It can be implemented at the AST-level, type information isn't needed.
It might be a good fit into either [`unused`](https://github.com/dominikh/go-tools/tree/master/cmd…
@ilius, not to my knowledge. It can be implemented at the AST-level, type information isn't needed.
It might be a good fit into either [`unused`](https://github.com/dominikh/go-tools/tree/master/cmd…shurcooL commented on unused exported function not report… in github.com/dominikh/go-tools1wTo be more precise, it’s a method. Its existence makes the `Abc` type implement the following interface:
```Go
type Func1er interface {
Func1()
}
```
If this were a library rather than command, …
To be more precise, it’s a method. Its existence makes the `Abc` type implement the following interface:
```Go
type Func1er interface {
Func1()
}
```
If this were a library rather than command, …shurcooL commented on exclude vendor dir from matching `.… in github.com/kisielk/errcheck1wThat's right. As of https://github.com/kisielk/gotool/issues/16 being resolved, `gotool` will not include `vendor` when expanding `...` on Go 1.9 and later.
That's right. As of https://github.com/kisielk/gotool/issues/16 being resolved, `gotool` will not include `vendor` when expanding `...` on Go 1.9 and later.shurcooL commented on Utilizing Same API Client Pattern i… in github.com/google/go-github1wAs Glenn said, Will is indeed the person who can give you a good answer.
I personally hope you do apply the best practices of making a high quality Go package to your API client library.
I've contr…
As Glenn said, Will is indeed the person who can give you a good answer.
I personally hope you do apply the best practices of making a high quality Go package to your API client library.
I've contr…shurcooL commented on Unmarshal data field only if presen… in github.com/shurcooL/graphql1wThanks for checking, @mwilli31.
Thanks for checking, @mwilli31.shurcooL commented on Panic when calling "getReader" on a… in github.com/neelance/go2wYeah, just checked latest Firefox Nightly, and Streams API is simply not supported (with default settings). Compare Firefox Nightly (62.0a1):
. Compare Firefox Nightly (62.0a1):
![image](https://user-images.githubusercontent.com/192…shurcooL commented on Panic when calling "getReader" on a… in github.com/neelance/go2wDid you check if the same functionality in JavaScript works?
Does that version of Firefox support streams API by now? https://caniuse.com/#search=ReadableStream Last time I checked (some time ago), …
Did you check if the same functionality in JavaScript works?
Does that version of Firefox support streams API by now? https://caniuse.com/#search=ReadableStream Last time I checked (some time ago), …shurcooL commented on Unexpected panic when using callback in github.com/neelance/go2wIt might be worthwhile to keep this open until @neelance gets a chance to see this.
I suspect there are API change opportunities to consider here, ones that could make the user experience better.
F…
It might be worthwhile to keep this open until @neelance gets a chance to see this.
I suspect there are API change opportunities to consider here, ones that could make the user experience better.
F…shurcooL commented on Unexpected panic when using callback in github.com/neelance/go2wAlso, have you tried this?
```diff
promise.Call("then", callback.New(func(args []js.Value) {
fmt.Println(args)
-}))
+}).Value)
```
Since `callback` package isn't documented yet, it's no…
Also, have you tried this?
```diff
promise.Call("then", callback.New(func(args []js.Value) {
fmt.Println(args)
-}))
+}).Value)
```
Since `callback` package isn't documented yet, it's no…shurcooL commented on Unexpected panic when using callback in github.com/neelance/go2w@johanbrandhorst This may or may not be related, bu the http://google.com URL doesn't permit CORS, so the code you tried to execute is expected to fail in browser:
 field tracks the information you need?
https://github.com/sourcegraph…
Can you check if the [`Hunk.OrigNoNewlineAt`](https://godoc.org/sourcegraph.com/sourcegraph/go-diff/diff#Hunk.OrigNoNewlineAt) field tracks the information you need?
https://github.com/sourcegraph…shurcooL commented on not call Flush can also print log r… in github.com/golang/gddo2wHi there,
This is an issue tracker for the gddo project, used to track bugs in it. People who may know the answer to your question are unlikely to see it here.
For asking questions about Go, se…
Hi there,
This is an issue tracker for the gddo project, used to track bugs in it. People who may know the answer to your question are unlikely to see it here.
For asking questions about Go, se…shurcooL commented on GraphQL Errors not be returned due … in github.com/shurcooL/graphql2w@mwilli31 I've sent PR #23 which should fix this issue. Could you give it a try and confirm it fixes it for you?
@mwilli31 I've sent PR #23 which should fix this issue. Could you give it a try and confirm it fixes it for you?shurcooL commented on Add checks api in github.com/google/go-github2w> Any recommended pattern to follow when a required field is dependent on another field.
Yes, we have no choice but to make them a pointer with `,omitempty`, and just rely on documentation.
See…
> Any recommended pattern to follow when a required field is dependent on another field.
Yes, we have no choice but to make them a pointer with `,omitempty`, and just rely on documentation.
See…shurcooL commented on GraphQL Errors not be returned due … in github.com/shurcooL/graphql2w@mwilli31 Can you share the GraphQL query and/or the Go code to execute that query that you used? I can't reproduce the problem yet.
@mwilli31 Can you share the GraphQL query and/or the Go code to execute that query that you used? I can't reproduce the problem yet.shurcooL commented on cmd/go: documentation unclear that … in github.com/golang/go2wI think [CL 109341](https://golang.org/cl/109341) (for issue #22726) has helped this issue somewhat (at least, it implemented a part of what I described in https://github.com/golang/go/issues/21529#i…
I think [CL 109341](https://golang.org/cl/109341) (for issue #22726) has helped this issue somewhat (at least, it implemented a part of what I described in https://github.com/golang/go/issues/21529#i…shurcooL commented on cmd/go: do nothing with explicitly … in github.com/golang/go2wThis is related to #21529.
This is related to #21529.shurcooL commented on GraphQL Errors not be returned due … in github.com/shurcooL/graphql2wThanks a lot for the information, it's very helpful.
I will work on a fix.
I suspect the right solution is that `UnmarshalGraphQL` should not return an error on that input. It should return nil err…
Thanks a lot for the information, it's very helpful.
I will work on a fix.
I suspect the right solution is that `UnmarshalGraphQL` should not return an error on that input. It should return nil err…shurcooL commented on Add checks api in github.com/google/go-github2wAnd this struct should be modified so the required parameters are values, not pointers. They also don't need `,omitempty` option. So:
```Go
// CreateCheckRunOptions sets up parameters need to cre…
And this struct should be modified so the required parameters are values, not pointers. They also don't need `,omitempty` option. So:
```Go
// CreateCheckRunOptions sets up parameters need to cre…shurcooL commented on Add checks api in github.com/google/go-github2wWas this line removed intentionally?
It doesn't look like an improvement, so unless you have a reason, I'd suggest reverting it.
Was this line removed intentionally?
It doesn't look like an improvement, so unless you have a reason, I'd suggest reverting it.shurcooL commented on GraphQL Errors not be returned due … in github.com/shurcooL/graphql2wHi, thanks for reporting.
> Minor bug where the response from the server is not compliant with the Query Struct and provides valid graphql Errors
I'd like to better understand when this can hap…
Hi, thanks for reporting.
> Minor bug where the response from the server is not compliant with the Query Struct and provides valid graphql Errors
I'd like to better understand when this can hap…shurcooL commented on Robust Header Check in github.com/bradleyfalzon/ghinstallation2wIt would be nice to document what the function does. Perhaps something like this:
```Go
// addAcceptHeader adds acceptHeader to headers, but only
// if the current "Accept" header is not set, or…
It would be nice to document what the function does. Perhaps something like this:
```Go
// addAcceptHeader adds acceptHeader to headers, but only
// if the current "Accept" header is not set, or…shurcooL commented on Robust Header Check in github.com/bradleyfalzon/ghinstallation2wWhat about the first part? I still see some comments that don't have a space after `//`.
What about the first part? I still see some comments that don't have a space after `//`.shurcooL commented on CHANGELOG.md: Fix typos in removed … in github.com/primer/octicons2wNo problem. I noticed that v7.3.0 was missing those 2 icons when upgrading from v6.0.1, so I wanted to check the CHANGELOG to confirm it was intentional. That’s when I spotted the typos.
No problem. I noticed that v7.3.0 was missing those 2 icons when upgrading from v6.0.1, so I wanted to check the CHANGELOG to confirm it was intentional. That’s when I spotted the typos.shurcooL pushed to CHANGELOG.md: Fix typos in removed icon names.
patch-1 in github.com/shurcooL/octicons-12wCHANGELOG.md: Fix typos in removed icon names.shurcooL commented on specify http username & password, a… in github.com/shurcooL/gtdo2w`https://go.googlesource.com/go` is known not to need authentication, so this shouldn't be done here.
`https://go.googlesource.com/go` is known not to need authentication, so this shouldn't be done here.shurcooL commented on Reduce memory use in github.com/gregjones/httpcache2wThe first commit (using `*{bytes,strings}.Reader` instead of `*bytes.Buffer`) looks good to me. It simplifies code and performs better.
Second and third commits have an inconsistency. Inside `Cach…
The first commit (using `*{bytes,strings}.Reader` instead of `*bytes.Buffer`) looks good to me. It simplifies code and performs better.
Second and third commits have an inconsistency. Inside `Cach…shurcooL commented on suggestion: Unify Foo and FooEXT fu… in github.com/go-gl/gl2w> I can use `GenFramebufersEXT` as a fallback of `GenFrambuffers` (or vise versa?), but should callers care of this? Can we have unified function, `GenFramebuffers` in this case, and always use them?…
> I can use `GenFramebufersEXT` as a fallback of `GenFrambuffers` (or vise versa?), but should callers care of this? Can we have unified function, `GenFramebuffers` in this case, and always use them?…shurcooL commented on Propagate context on token refresh … in github.com/bradleyfalzon/ghinstallation2w> Yeah I added the new method to avoid making a breaking API change. Should I just add the context argument to the Token() method?
Here's what I wrote in #8 about this:
> Given the breaking cha…
> Yeah I added the new method to avoid making a breaking API change. Should I just add the context argument to the Token() method?
Here's what I wrote in #8 about this:
> Given the breaking cha…shurcooL commented on carddav: Remove duplicate package c… in github.com/emersion/go-webdav2wThank you for the quick review. :)
Thank you for the quick review. :)shurcooL commented on Question: Should we change the lern… in github.com/primer/octicons2wGiven #208 and #220 have been merged, should this issue be closed now? Or is there more to be done here?
Given #208 and #220 have been merged, should this issue be closed now? Or is there more to be done here?shurcooL pushed to carddav: Remove duplicate package comment
patch-1 in github.com/shurcooL/go-webdav2wcarddav: Remove duplicate package commentshurcooL commented on Expose the client's BaseURL. in github.com/andygrunwald/go-gerrit2wThere are no outstanding review comments, and this is ready from my side. Merging (preserving individual commits, since they're logical, independent, with descriptive commit messages).
There are no outstanding review comments, and this is ready from my side. Merging (preserving individual commits, since they're logical, independent, with descriptive commit messages).shurcooL commented on Expose the client's BaseURL. in github.com/andygrunwald/go-gerrit2w> I forgot the fact that baseURL of the client struct is not exported and only accessible through the getter function.
As of this PR. It wasn't accessible before, which is what I wanted to fix. :)
> I forgot the fact that baseURL of the client struct is not exported and only accessible through the getter function.
As of this PR. It wasn't accessible before, which is what I wanted to fix. :)