Yesterday
dmitshur commented on x/build/cmd/gopherbot: assignReview… in github.com/golang/go17h
> we can look into adding a workaround for it now by modifying `humanReviewersOnChange` to not consider authors of the CL as reviewers.
Doing that is proving to be difficult because of issue #2396…
> we can look into adding a workaround for it now by modifying `humanReviewersOnChange` to not consider authors of the CL as reviewers.
Doing that is proving to be difficult because of issue #2396…dmitshur commented on x/build/cmd/gerritbot: Gerrit asks … in github.com/golang/go18hThis is causing #31658.
@andybons Is the latest status still as before? In that case, I plan to work around it on our side by writing extra code in `humanReviewersOnChange` to filter out the CL au…
This is causing #31658.
@andybons Is the latest status still as before? In that case, I plan to work around it on our side by writing extra code in `humanReviewersOnChange` to filter out the CL au…This Week
dmitshur commented on crypto/ssh: Document that ParseRawP… in go.googlesource.com/crypto1d
(1 comment)
Hi, thanks for the CL.
Is there an issue accompanying this CL, or is it small enough that it didn't need an issue?
The prefix shouldn't include the subrepo name, just the directory ins…
(1 comment)
Hi, thanks for the CL.
Is there an issue accompanying this CL, or is it small enough that it didn't need an issue?
The prefix shouldn't include the subrepo name, just the directory ins…dmitshur commented on net/url: Host/Port extraction malfu… in github.com/golang/go1d/cc @rsc @bradfitz (per `net/url` [owners](https://dev.golang.org/owners))
/cc @rsc @bradfitz (per `net/url` [owners](https://dev.golang.org/owners))dmitshur commented on net/url: clarify in docs that RawPa… in github.com/golang/go1d/cc @rsc @bradfitz (per `net/url` [owners](https://dev.golang.org/owners))
/cc @rsc @bradfitz (per `net/url` [owners](https://dev.golang.org/owners))
dmitshur commented on net/http: StripPrefix regression fr… in github.com/golang/go2dI think the original snippet be simplified to https://play.golang.org/p/qIYFprNaibv.
I think the original snippet be simplified to https://play.golang.org/p/qIYFprNaibv.dmitshur commented on net/http: StripPrefix regression fr… in github.com/golang/go2dI suspect this is an unfortunate but intended consequence of fixing #30165. Code relying on that should be updated not to rely on it. That is, instead of:
```Go
http.Handle("/foo/", http.StripPre…
I suspect this is an unfortunate but intended consequence of fixing #30165. Code relying on that should be updated not to rely on it. That is, instead of:
```Go
http.Handle("/foo/", http.StripPre…dmitshur commented on context: made a decision whether to… in github.com/golang/go2dAnswered with more detail in a direct message, but to summarize: my findings show very few such tests.
It's also worth pointing out that there is no `String` method in the public API of the `context…
Answered with more detail in a direct message, but to summarize: my findings show very few such tests.
It's also worth pointing out that there is no `String` method in the public API of the `context…dmitshur commented on context: don't depend on fmt in go.googlesource.com/go2d(1 comment)
> let's revisit this
I didn't find an issue tracking that, so made issue #31620.
There were some tests that started to fail on Go tip due to this change, and the next steps there depen…
(1 comment)
> let's revisit this
I didn't find an issue tracking that, so made issue #31620.
There were some tests that started to fail on Go tip due to this change, and the next steps there depen…dmitshur commented on cmd/go/internal/modcmd: allow mod d… in go.googlesource.com/go2d(1 comment)
Friendly ping Bryan, PTAL.
I considered suggesting adding
! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.2.1.zip
here, but I suspect it's not worth it. We should feel …
(1 comment)
Friendly ping Bryan, PTAL.
I considered suggesting adding
! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.2.1.zip
here, but I suspect it's not worth it. We should feel …dmitshur commented on cmd/go/internal/modcmd: allow mod d… in go.googlesource.com/go2d(1 comment)
Did you see this comment? Just checking that it wasn’t missed.
(1 comment)
Did you see this comment? Just checking that it wasn’t missed.dmitshur commented on cmd/go/internal/modcmd: allow mod d… in go.googlesource.com/go2d> 2 of 20 TryBots failed [...] Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
The …
> 2 of 20 TryBots failed [...] Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
The …dmitshur commented on quickedit: constant 1-byte substr i… in github.com/dominikh/go-tools3dAs a data point, I wrote this today:
```Go
i := bytes.LastIndex(text, []byte("#"))
```
By chance, I realized I could use `bytes.LastIndexByte` to simplify it:
```Go
i := bytes.LastIndexBy…
As a data point, I wrote this today:
```Go
i := bytes.LastIndex(text, []byte("#"))
```
By chance, I realized I could use `bytes.LastIndexByte` to simplify it:
```Go
i := bytes.LastIndexBy…dmitshur commented on 3.3 beta - don't merge yet in github.com/go-gl/glfw3d> Got rid of internal error callback. Never liked the way we solved it back then. It was not idiomatic and it was not mirroring the C API (worst of both worlds?). Now we have glfwGetError() function …
> Got rid of internal error callback. Never liked the way we solved it back then. It was not idiomatic and it was not mirroring the C API (worst of both worlds?). Now we have glfwGetError() function …dmitshur commented on looks like 3.3 is officially out in github.com/go-gl/glfw3dThere's an open PR #219. It needs to be reviewed, then it can be merged.
There's an open PR #219. It needs to be reviewed, then it can be merged.dmitshur commented on cmd/go/internal/modcmd: allow mod d… in go.googlesource.com/go3d(1 comment)
Also, you can drop the "golang/go" prefix, since this CL is in the main repository. Just:
Fixes #29522
(1 comment)
Also, you can drop the "golang/go" prefix, since this CL is in the main repository. Just:
Fixes #29522dmitshur reviewed +1 on cmd/go/internal/modcmd: allow mod d… in go.googlesource.com/go3d(2 comments)
This didn't make it into 1.12, but we should get it into 1.13.
s/Fix/Fixes/ per https://golang.org/wiki/CommitMessage.
I'm not sure if the local variable pkgMod is helping with readab…
(2 comments)
This didn't make it into 1.12, but we should get it into 1.13.
s/Fix/Fixes/ per https://golang.org/wiki/CommitMessage.
I'm not sure if the local variable pkgMod is helping with readab…dmitshur commented on shiny/driver/mtldriver: add start o… in go.googlesource.com/exp3dI've addressed all the minor comments. What's left is reworking the main GLFW loop to run in a separate goroutine on the main thread and fixing the NextEvent implementation accordingly.
I've addressed all the minor comments. What's left is reworking the main GLFW loop to run in a separate goroutine on the main thread and fixing the NextEvent implementation accordingly.dmitshur commented on shiny/driver/mtldriver: add start o… in go.googlesource.com/exp3d(6 comments)
Answered your question, marking as resolved.
Done is PS 7.
Marking this resolved unless you have further comments.
Ended up going with a func toCBool(b bool) C.BOOL helper.
Done in …
(6 comments)
Answered your question, marking as resolved.
Done is PS 7.
Marking this resolved unless you have further comments.
Ended up going with a func toCBool(b bool) C.BOOL helper.
Done in …dmitshur commented on shiny/driver/mtldriver: add start o… in go.googlesource.com/exp3d(2 comments)
Done in PS 6.
Done in PS 6.
(2 comments)
Done in PS 6.
Done in PS 6.Last Week
dmitshur commented on cmd/go: mod meta tag causes infinit… in github.com/golang/go5d@marwan-at-work One of the use cases where the `mod` go-import meta tag feature seems to work well is for publishing modules that do not have a counter-part public VCS.
For example, the module `dm…
@marwan-at-work One of the use cases where the `mod` go-import meta tag feature seems to work well is for publishing modules that do not have a counter-part public VCS.
For example, the module `dm…dmitshur commented on Use Bitbucket API v2 in github.com/golang/gddo5dgodoc.org has been redeployed with the fix ([CL 170279](https://golang.org/cl/170279)) now. /cc @arnottcr
godoc.org has been redeployed with the fix ([CL 170279](https://golang.org/cl/170279)) now. /cc @arnottcrdmitshur commented on encoding/gob: test frequently faili… in github.com/golang/go5d/cc @ianthehat FYI. It seems you ran into a similar issue in [CL 172778](https://golang.org/cl/172778#message-28b4232a2f9471641d9230bf2ab06af5257941ba).
/cc @ianthehat FYI. It seems you ran into a similar issue in [CL 172778](https://golang.org/cl/172778#message-28b4232a2f9471641d9230bf2ab06af5257941ba).dmitshur commented on access: Trybot Access ("may-start-t… in github.com/golang/go5dHi @bsiegert,
Thank you for contributing to the Go project and helping maintain some of the builders! I've discussed this request with the team (/cc @golang/osp-team), and we agreed to grant you a…
Hi @bsiegert,
Thank you for contributing to the Go project and helping maintain some of the builders! I've discussed this request with the team (/cc @golang/osp-team), and we agreed to grant you a…dmitshur reviewed +2 on old/netchan: disable a test with a … in go.googlesource.com/exp5d(1 comment)
This isn't using any formatting verbs, so t.Skip would suffice.
(Feel free to leave as is if you don't think that t.Skip is slightly simpler than t.Skipf.)
(1 comment)
This isn't using any formatting verbs, so t.Skip would suffice.
(Feel free to leave as is if you don't think that t.Skip is slightly simpler than t.Skipf.)dmitshur commented on x/build: run github.com/golang/gddo… in github.com/golang/go5d@bradfitz I've remembered that we also have post-submit builders via Travis CI on that repo. I.e., https://travis-ci.org/golang/gddo/builds.
@bradfitz I've remembered that we also have post-submit builders via Travis CI on that repo. I.e., https://travis-ci.org/golang/gddo/builds.dmitshur commented on gosrc: use Bitbucket v2 api in go.googlesource.com/gddo5d(1 comment)
Thank you!
Ok, let's proceed without it for now. Adding support for watchers via Bitbucket API v2 can be an optional followup feature. The scope of this CL is to migrate away from API v…
(1 comment)
Thank you!
Ok, let's proceed without it for now. Adding support for watchers via Bitbucket API v2 can be an optional followup feature. The scope of this CL is to migrate away from API v…dmitshur commented on internal/lsp: extract the common te… in go.googlesource.com/tools5dWorth noting is that neither Go 1.12 nor 1.11 release branch trybots failed, only the Go tip ones. That means it's possible that latest changes on tip are involved, not just changes in x/tools. (Or i…
Worth noting is that neither Go 1.12 nor 1.11 release branch trybots failed, only the Go tip ones. That means it's possible that latest changes on tip are involved, not just changes in x/tools. (Or i…dmitshur commented on cmd/go: only add a 'go' directive o… in go.googlesource.com/go5d(1 comment)
I realize it's already long, but given how important the behavior is, I think it's worth explicitly adding to the commit subject that 'go mod tidy' also adds the 'go' directive. The curr…
(1 comment)
I realize it's already long, but given how important the behavior is, I think it's worth explicitly adding to the commit subject that 'go mod tidy' also adds the 'go' directive. The curr…dmitshur commented on sourcegraph.com/sourcegraph/go-diff… in github.com/sourcegraph/go-diff5dThanks for reporting. This is an issue that needs to be fixed in the github.com/johnwyles/vrddt-reboot project. It needs to start importing `go-diff` via its new import path `github.com/sourcegraph/g…
Thanks for reporting. This is an issue that needs to be fixed in the github.com/johnwyles/vrddt-reboot project. It needs to start importing `go-diff` via its new import path `github.com/sourcegraph/g…dmitshur reviewed +2 on dashboard: don't build x/net master… in go.googlesource.com/build5d(1 comment)
Add a link to the issue comment here, so it'll be more clear why this behavior is here when reading the code later.
(1 comment)
Add a link to the issue comment here, so it'll be more clear why this behavior is here when reading the code later.dmitshur commented on image/gif: transparency lost when e… in github.com/golang/go6dThe concrete type returned by `image.Decode` in your snippet is `*image.Paletted`.
I suspect this issue is happening because wrapping the `*image.Paletted` in a concrete type such as `offsetImage`…
The concrete type returned by `image.Decode` in your snippet is `*image.Paletted`.
I suspect this issue is happening because wrapping the `*image.Paletted` in a concrete type such as `offsetImage`…dmitshur commented on image/gif: transparency lost when e… in github.com/golang/go6d@splace Thanks for the issue report.
When you say "images the same" under "what did you expect", can you clarify? There are 4 images, which ones did you expect to be the same?
@splace Thanks for the issue report.
When you say "images the same" under "what did you expect", can you clarify? There are 4 images, which ones did you expect to be the same?dmitshur commented on crypto/tls: func (c *Conn) Handshak… in github.com/golang/go6d/cc @FiloSottile
/cc @FiloSottiledmitshur commented on os/signal: TestAtomicStop fails on … in github.com/golang/go6d/cc @ianlancetaylor as owner of `os/signal`.
There is a lot of red for dragonfly/amd64 on build.golang.org on tip (almost half are fail). I've checked 5 pages worth and none of them are `TestAtomi…
/cc @ianlancetaylor as owner of `os/signal`.
There is a lot of red for dragonfly/amd64 on build.golang.org on tip (almost half are fail). I've checked 5 pages worth and none of them are `TestAtomi…dmitshur commented on misc: add tests to work with modules in github.com/golang/go6d/cc @bcmills I think you may have addressed this fully or partially during the Go 1.13 dev cycle. Please see if this issue needs to be updated.
/cc @bcmills I think you may have addressed this fully or partially during the Go 1.13 dev cycle. Please see if this issue needs to be updated.dmitshur commented on crypto/tls: add heartbeat extension… in github.com/golang/go6d/cc @FiloSottile Please see if this issue from 2014 is still relevant.
/cc @FiloSottile Please see if this issue from 2014 is still relevant.dmitshur commented on cmd/doc: go doc rsc.io/quote/v3 in … in github.com/golang/go1wThere's a chance this is the same issue as https://github.com/golang/go/issues/28992#issuecomment-483441942.
There's a chance this is the same issue as https://github.com/golang/go/issues/28992#issuecomment-483441942.dmitshur commented on cmd/doc: 'go doc $pkg' uses GOPATH … in github.com/golang/go1wI tested with the following steps, using Go 1.12.4:
```
$ export GO111MODULE=on
$ export GOPATH="$(mktemp -d)"
$ cd $(mktemp -d)
$ go mod init m
$ go get rsc.io/quote/v3
$ go get dmitri.shur…
I tested with the following steps, using Go 1.12.4:
```
$ export GO111MODULE=on
$ export GOPATH="$(mktemp -d)"
$ cd $(mktemp -d)
$ go mod init m
$ go get rsc.io/quote/v3
$ go get dmitri.shur…dmitshur commented on Rename of repository not handled we… in github.com/sourcegraph/go-diff1w> The obvious fix is to just use the new github.com import path, but this is not always trivial. In particular, when go-diff is pulled in by a dependency that has the old url, running go get -u to up…
> The obvious fix is to just use the new github.com import path, but this is not always trivial. In particular, when go-diff is pulled in by a dependency that has the old url, running go get -u to up…dmitshur commented on Rename of repository not handled we… in github.com/sourcegraph/go-diff1w@matthijskooijman Looks like the latest version of `github.com/gobuffalo/buffalo-plugins` (v1.14.0) module needs to update its dependencies. Here's the chain of modules being required:
```
go get…
@matthijskooijman Looks like the latest version of `github.com/gobuffalo/buffalo-plugins` (v1.14.0) module needs to update its dependencies. Here's the chain of modules being required:
```
go get…dmitshur commented on Add go.mod in github.com/go-gl/mathgl1w> What's lost on me is how you could tell that I didn't know
It was a guess based on that you picked version 1.11 rather than 1.12, given we're trying to support Go 1.12 and 1.11. A common misconc…
> What's lost on me is how you could tell that I didn't know
It was a guess based on that you picked version 1.11 rather than 1.12, given we're trying to support Go 1.12 and 1.11. A common misconc…dmitshur commented on vfsgendev updates modification times in github.com/gopherjs/gopherjs1wThe comments above accurately reflect the current status. There hasn't been progress made since then.
The comments above accurately reflect the current status. There hasn't been progress made since then.dmitshur commented on Change import path to github.com/ug… in github.com/ugorji/go1w> travis-ci, etc do not work well with a repo name different from the import path
Travis CI supports this well via the `go_import_path` field. See https://docs.travis-ci.com/user/languages/go/#go-…
> travis-ci, etc do not work well with a repo name different from the import path
Travis CI supports this well via the `go_import_path` field. See https://docs.travis-ci.com/user/languages/go/#go-…dmitshur commented on 
🚀 in github.com/gbbr/breakcheck1w`apidiff` has semi-graduated from x/exp to x/tools:
[golang.org/x/tools/internal/apidiff](https://github.com/golang/tools/tree/master/internal/apidiff)
🚀 in github.com/gbbr/breakcheck1w`apidiff` has semi-graduated from x/exp to x/tools:
[golang.org/x/tools/internal/apidiff](https://github.com/golang/tools/tree/master/internal/apidiff)dmitshur commented on Support preview GraphQL API v4 Node… in github.com/google/go-github1wAs far as I can tell, this field doesn't need to be added to `NewTeam`. `NewTeam` is used when creating or editing a team, but neither of those two endpoints accept a `node_id` parameter:
- https:…
As far as I can tell, this field doesn't need to be added to `NewTeam`. `NewTeam` is used when creating or editing a team, but neither of those two endpoints accept a `node_id` parameter:
- https:…dmitshur commented on compiler/natives/src/time: disable … in github.com/gopherjs/gopherjs1wMinor, but it would be nicer to merge a small PR like this via the "squash and merge" strategy, so it would generate just 1 commit on master branch rather than two. This is because we're trying to fo…
Minor, but it would be nicer to merge a small PR like this via the "squash and merge" strategy, so it would generate just 1 commit on master branch rather than two. This is because we're trying to fo…Earlier
dmitshur commented on Update Travis to run with supported… in github.com/go-gl/mathgl1w`go test` runs a subset of of vet checks, so it’s still useful to run vet. It should be done this way now:
```
- go vet ./...
```
`go test` runs a subset of of vet checks, so it’s still useful to run vet. It should be done this way now:
```
- go vet ./...
```dmitshur commented on Update Travis to run with supported… in github.com/go-gl/mathgl1wWe can simplify this to always use the latest patch versions and not having to update the travis config when new ones come out:
```
- "1.12.x"
- "1.11.x"
```
We can simplify this to always use the latest patch versions and not having to update the travis config when new ones come out:
```
- "1.12.x"
- "1.11.x"
```dmitshur commented on Add go.mod in github.com/go-gl/mathgl1w> Support go1.11 onwards.
I want to point out that this directive means to use the Go language version 1.11; it’s not for setting the minimum version of Go that is supported. Letting you know in…
> Support go1.11 onwards.
I want to point out that this directive means to use the Go language version 1.11; it’s not for setting the minimum version of Go that is supported. Letting you know in…dmitshur commented on go.mod: update dependencies in github.com/googleapis/google-api-go-client2w@jamesabbottsmith I believe it was a typo. It should say v0.3.2 for `google.golang.org/api`. The other two versions look correct. Of course, newer tags are okay to use too.
@jamesabbottsmith I believe it was a typo. It should say v0.3.2 for `google.golang.org/api`. The other two versions look correct. Of course, newer tags are okay to use too.dmitshur commented on Add ForceAllTypes to Options in github.com/shurcooL/vfsgen2wSorry about the delays.
I am leaning towards not merging this PR as is because of the increased difficulty of maintenance, and I don't have the bandwidth to spend on this now. However, I do want t…
Sorry about the delays.
I am leaning towards not merging this PR as is because of the increased difficulty of maintenance, and I don't have the bandwidth to spend on this now. However, I do want t…dmitshur commented on WIP: Generate separate data and com… in github.com/shurcooL/vfsgen2wI would like to complete the general idea started here, but it will happen no earlier than a few weeks from now, and I plan to start relying on modules to implement the idea. It may make sense to mak…
I would like to complete the general idea started here, but it will happen no earlier than a few weeks from now, and I plan to start relying on modules to implement the idea. It may make sense to mak…dmitshur commented on Expose constructQuery to public fun… in github.com/shurcooL/graphql2wI'm warming up to this idea, but in a slightly different form.
I think it might be a good idea to expose some of the graphql internals (in this project), but in another location. The idea would be…
I'm warming up to this idea, but in a slightly different form.
I think it might be a good idea to expose some of the graphql internals (in this project), but in another location. The idea would be…dmitshur commented on Provide errors response for error h… in github.com/shurcooL/githubv42w> I think actually shurcooL/graphql#31 or rather shurcooL/graphql#33 should fix this and this library here doesn't need to change much.
Agreed.
> Is there anything we can do to help bring this …
> I think actually shurcooL/graphql#31 or rather shurcooL/graphql#33 should fix this and this library here doesn't need to change much.
Agreed.
> Is there anything we can do to help bring this …dmitshur commented on Fix function comments based on best… in github.com/go-gl/mathgl2wHere and below as well.
Here and below as well.dmitshur commented on Fix function comments based on best… in github.com/go-gl/mathgl2wCan you change "Does" to "does" here, and everywhere similar below?
Can you change "Does" to "does" here, and everywhere similar below?