Yesterday
dmitshur commented on golang.org/x/exp/shiny/driver/mtldriver: add start of a Metal API-based driver6h
(4 comments) Thank you for the thorough review Nigel! There's still much unrealized potential in this Metal-backed driver (like, actually doing rendering on the GPU instead of on the CPU), but I th…
dmitshur pushed to master in github.com/shurcooL/gtdo7h
dbf8d1d0e13feb9795605c5c83929c987605e3e4fix summary <pre> font family and background color in Dark Mode
dmitshur commented on golang.org/x/website/cmd/golangorg: inconsistent tabwidth value10h
I would prefer going with 4 everywhere, because 8 is quite wide, and 4 seems more commonly used. The future discovery site (issue #33654) and godoc.org currently use a tab width of 4: https://godo…
dmitshur opened an issue golang.org/x/website/cmd/golangorg: inconsistent tabwidth value10h
https://tip.golang.org uses the [default godoc presentation](https://github.com/golang/tools/blob/5eefd052ad727afc5e2e7b034c752b9d3d902b3b/godoc/pres.go#L106) tab width of 4, but https://golang.org u…
dmitshur commented on google.golang.org/grpc/proto codegen: Deprecated comments not generated for methods12h
PR https://github.com/golang/protobuf/pull/952 was merged. Is this issue resolved, or is there more to do here?
dmitshur starred in github.com/TrueFurby/goexplorer12h
dmitshur deleted branch in github.com/mdempsky/unconvert13h
travis-go13
dmitshur pushed to master in github.com/mdempsky/unconvert13h
3ecd357795aff6024d9b038acc536bb7090c4e2funconvert: update Travis for Go 1.13
dmitshur commented on github.com/mdempsky/unconvert/unconvert: fix calling without arguments13h
Thank you for fixing this Egon!
dmitshur created branch in github.com/mdempsky/unconvert13h
travis-go13
dmitshur pushed to master in github.com/mdempsky/unconvert13h
b134f719ea9b1ed9d097b2bf24d534e4aa7166fdunconvert: make no arguments mean current directory
dmitshur pushed to fix-no-args in github.com/mdempsky/unconvert13h
06727a0fc6ca24bef61b85cdffa7a5bea8ff8328unconvert: rename workdir struct field to dir
dmitshur commented on github.com/yifanwu/blanknewtab: Question about the extension button.13h
> Or is the source of the published extension (i.e., https://chrome.google.com/webstore/detail/blank-new-tab/beafekehjfhnkpnnjegadfdncaipnljp) slightly different than what's in this repo? The exte…
Last Week
dmitshur commented on github.com/gopherjs/gopherjs: support for being built and used with newer version of Go1d
I edited the wording to say that approach may not be working today.
dmitshur commented on github.com/gopherjs/gopherjs: support for being built and used with newer version of Go1d
I see. Do you think it'd be easier if this issue gets accepted? That way, all that you'll need to do will be to set the new `GOPHERJS_GOROOT` environment variable to a correct value. You can try it o…
dmitshur commented on github.com/gopherjs/gopherjs: support for being built and used with newer version of Go1d
Try building a program like [this](https://play.golang.org/p/c6tJaEBhIFf) using `go1.12.9` and see what value it reports. In order for it to find the GOROOT where `go1.12.9` is located, the `GOROOT` …
dmitshur commented on golang.org/x/build/cmd/gopherbot: update link to troubleshooting page1d
(1 comment) Another option is to use the markdown rendering on gitiles (e.g., https://go.googlesource.com/tools/+/refs/heads/master/gopls/doc/troubleshooting.md), which is the canonical source and …
dmitshur reviewed +2 on golang.org/x/build/cmd/gopherbot: update link to troubleshooting page1d
(1 comment) If you want, you can consider adding a fragment like https://github.com/golang/tools/blob/master/gopls/doc/troubleshooting.md#readme or https://github.com/golang/tools/blob/master/gopls/…
dmitshur commented on runtime/debug: BuildInfo lacks information about “the main module”2d
@icholy The term "main module" is described in the first sentence at https://golang.org/cmd/go/#hdr-The_main_module_and_the_build_list.
dmitshur reviewed +1 on runtime/debug: correct BuildInfo.Main documentation2d
(2 comments) Minor, per https://golang.org/wiki/CommitMessage and https://tip.golang.org/doc/contribute.html#commit_messages this should be s/Update/Updates/ and should go after the commit body, not…
dmitshur commented on golang.org/x/build: sharded iOS builders2d
@bradfitz @eliasnaur Do you know if there are there known limitations on the `android-amd64-emu` builder type? I checked its notes in [`dashboard`](https://github.com/golang/build/blob/6d21d91eff340b…
dmitshur starred in github.com/bradfitz/inboxfewer3d
dmitshur commented on golang.org/x/build/internal/gophers: restore valid Gerrit emails (again)3d
(3 comments) Talked with Andy, he asked me to keep all values and squash into one addPerson line. (He can always follow up if he wants to adjust it further.) Done. Done. I'll leave the other Dmit…
dmitshur commented on go/doc: lines ending with parenthesis cannot be headings3d
Thank you for reporting. The implementation contains a rule that a heading "must end in a letter or digit", which is preventing a heading that ends with ')' to become a title. See: https://gith…
dmitshur opened an issue golang.org/x/review/git-codereview: change NNNN[/PP] feature isn't mentioned in package documentation3d
Running `git codereview help` mentions the following way to use the `change` command: ``` change NNNN[/PP] Checkout the commit corresponding to CL number NNNN and patch set PP from Gerrit.…
dmitshur commented on golang.org/x/net/http2/h2demo: fix the HTTP/1-vs-HTTP/2 demo after HSTS breakage3d
(1 comment) Thanks for answering and providing context.
dmitshur commented on github.com/mdempsky/unconvert/unconvert: fix calling without arguments3d
You can also add a comment like: ```suggestion patterns := flag.Args() // 0 or more import path patterns. ``` That'll make it more clear that it can be an empty slice, and that is okay.
dmitshur reviewed +1 on github.com/mdempsky/unconvert/unconvert: fix calling without arguments3d
Thanks for working on this! Looks good to me, but should also be reviewed by Matthew. Consider renaming this variable to `patterns`, here, as well as in the functions `mergeEdits` and `computeEdi…
dmitshur commented on golang.org/x/build/env/linux-mips: we have no Linux MIPS builders3d
@mengzhuo FYI, I've merged the [change](https://golang.org/cl/195977#message-93a5a2b686935fc5a8a617e90c69e1aca60f9804) that sets `SkipSnapshot` to true and deployed coordinator (an hour~ ago).
dmitshur commented on golang.org/x/net/http2/h2demo: fix the HTTP/1-vs-HTTP/2 demo after HSTS breakage3d
(1 comment) Why is this redirect added? It makes it not possible to view the index page via HTTP/1. I wanted to be able to compare http2.golang.org and http1.golang.org to see "Congratulations, you…
dmitshur commented on golang.org/x/net/http2/h2demo: remove h2demo build constraint3d
Deployed h2demo to confirm it still works, no problems. Submitting.
dmitshur reviewed +2 on golang.org/x/build/dashboard: skipsnapshot for linux-mipsle-mengzhuo builder3d
I share Brad's concerns at https://golang.org/issue/31217#issuecomment-532258895, but let's give this a try. It's a step forward, let's see how it goes.
dmitshur commented on golang.org/x/net/http2/h2demo: has started to cause x/net builders to fail4d
I prototyped the idea of removing the `h2demo` build constraint in [CL 196036](https://golang.org/cl/196036). It passed all trybots except 1.12.x one, because that one ran tests in GOPATH mode, and f…
dmitshur opened an issue golang.org/x/net/http2/h2demo: has started to cause x/net builders to fail4d
Issue #32528 has been resolved and all inner modules in x/net are being tested now, which includes the `golang.org/x/net/http2/h2demo` module. It contains a single package, which is a command with an…
dmitshur commented on golang.org/x/exp/errors/fmt: TestPanics broken4d
I'm not familiar enough with this code to know how the test should be fixed. I can send a CL to skip this test on builders with a reference to this issue. It'd be good to have a fix sooner so that tr…
dmitshur commented on golang.org/x/exp/errors/fmt: TestPanics broken4d
Issue #32528 has been resolved, and so the tests for the `golang.org/x/exp/errors` module will be executed when trybots run. That means this test failure will affect all trybot runs in x/exp.
dmitshur commented on golang.org/x/build/cmd/coordinator: simplify fetchDependenciesToGOPATHWorkspace4d
(2 comments) Done. I tried it, but it didn't work out great. It increased complexity because there was added skew between toProcess and seen. In the current version, seen is just a map of all entri…
dmitshur commented on golang.org/x/build/cmd/coordinator: simplify fetchDependenciesToGOPATHWorkspace4d
(1 comment) Thanks for the suggestion. I thought about this some more, and it might be better to just factor the findDeps(st.SubName) call out of the for loop. That should simplify the loop and rem…
dmitshur commented on golang.org/x/build/maintner/cmd/maintserve: break the build4d
This CL has served its purpose. Issue #32528 is now fixed.
dmitshur merged a change golang.org/x/build/crfs: delete4d
dmitshur commented on golang.org/x/build/crfs: delete4d
Acknowledging failing trybots on this commit, but submitting this together with CL 196018, where trybots are passing.
dmitshur commented on golang.org/x/build/cmd/coordinator: sum.golang.org is blocked by GO_DISABLE_OUTBOUND_NETWORK=14d
I've looked, and indeed, the firewall is being restricted very early on, when the `bc.Exec` call is made to invoke `go env -json GOMOD` here: https://github.com/golang/build/blob/da876aaec3965bc39…
dmitshur commented on golang.org/x/build/crfs: delete4d
> Well, these trybots aren't for CL 196018. They're for 71b4f49 exactly, and at this commit there are tidy errors, modifying go.sum, which means it'll be hitting sum.golang.org (and getting denied). …
dmitshur commented on golang.org/x/build/crfs: delete4d
> I think you need a go mod tidy in the maintserve module. > > It's hitting sum.golang.org and getting denied. It should already be tidy (after CL 196018), I think its replace directive is getting …
dmitshur opened an issue golang.org/x/build/cmd/coordinator: sum.golang.org is blocked by GO_DISABLE_OUTBOUND_NETWORK=14d
The fix to #32528 was implemented by finding nested modules, and then invoking `go test` once for each one, in sequence. ~~This isn't compatible with `GO_DISABLE_OUTBOUND_NETWORK=1` being set and …
dmitshur commented on golang.org/x/build/crfs: delete4d
This should not be failing trybots. I think I know why it is failing. The tests for nested modules run in sequence. When the first module is tested, GO_DISABLE_OUTBOUND_NETWORK=1 makes it permanentl…
dmitshur opened a change golang.org/x/build/crfs: delete4d
dmitshur commented on golang.org/x/build/maintner/cmd/maintserve: remove replace directive, add go directive4d
The problem is in the https://github.com/golang/build/tree/master/crfs module. It requires bazil.org/fuse@v0.0.0-20180421153158-65cc252bf669, but that package doesn't support openbsd and windows. Th…
dmitshur commented on golang.org/x/build/maintner/cmd/maintserve: remove replace directive, add go directive4d
I'm looking into it. Possibly. It's an error building bazil.org/fuse on those targets: # bazil.org/fuse ../../../../../pkg/mod/bazil.org/fuse@v0.0.0-20180421153158-65cc252bf669/error_std.go:27:20: u…
dmitshur commented on golang.org/x/build/cmd/coordinator: simplify fetchDependenciesToGOPATHWorkspace4d
Let me provide further motivation for this change to explain why I think it's good. When I read the original code, I find it hard to be confident that it has the expected behavior, and I find it har…
dmitshur commented on golang.org/x/build/cmd/coordinator: test packages in nested modules (in multi-module repositories)4d
I initially posted the above comment in the wrong issue at https://github.com/golang/go/issues/34247#issuecomment-532228755. I've moved it here, but that issue has some additional discussion. /cc @bc…
dmitshur commented on golang.org/x/build/cmd/coordinator: test packages in nested modules (in multi-module repositories)4d
I've tested the coordinator change for this issue on x/build [CL 191018](https://golang.org/cl/191018) and x/tools [CL 191018](https://golang.org/cl/191018) by restarting their trybots just now, and …
dmitshur commented on golang.org/x/build/cmd/coordinator: reduce nesting in buildStatus.runSubrepoTests4d
> while correctly ignoring other go.mod files in testdata directories: Also, I should clarify, what I meant there was "as intended". It's not that one way or another is correct, there are differen…
dmitshur commented on golang.org/x/build/env/linux-mips: we have no Linux MIPS builders4d
Indeed, it isn't working for your builder even after coordinator has been deployed with TLS 1.2. I noticed that it _is_ working for other builder types. For example, I watched [this trybot run](ht…
dmitshur commented on golang.org/x/build/cmd/coordinator: reduce nesting in buildStatus.runSubrepoTests4d
That's a fair point. But I think that's a problem that needs to be addressed by the module authors themselves. The scope of this issue is about increasing the test coverage of golang.org/x repos t…
dmitshur opened an issue golang.org/x/build: add x/tools trybot for main Go repository4d
As @jayconrod suggested in https://github.com/golang/go/issues/34321#issuecomment-531885130, I think we should add an x/tools trybot builder for the main Go repository. It would check whether x/tools…
dmitshur commented on golang.org/x/build: optional auto-submit on +2 when trybots (with rebase) pass4d
/cc @FiloSottile who also wanted this.
dmitshur commented on golang.org/x/build: enhance trybot's successful run report4d
I'll fold this into #34119 if you don't mind. We've decided there this is something we want to implement, so it's just a matter of time.
dmitshur commented on golang.org/x/build/cmd/coordinator: reduce nesting in buildStatus.runSubrepoTests4d
@bcmills I'm aware that go.mod files in testdata directories create module boundaries. I've discussed it with @ianthehat and we concluded that such modules have module paths unsuitable for externa…
dmitshur commented on golang.org/x/tools/gopls: use go-diff for edit generation4d
> All other trybot failures are due to a cmd/go change on tip, issue #34321 is tracking that. That issue has been fixed by now. > The Go 1.13.x failure is due to it not skipping "testdata" director…
dmitshur commented on golang.org/x/build/cmd/coordinator: reduce nesting in buildStatus.runSubrepoTests4d
**Edit:** Moved this comment to the right issue, see https://github.com/golang/go/issues/32528#issuecomment-532299794.
dmitshur commented on golang.org/x/build/env/linux-mips: we have no Linux MIPS builders4d
@mengzhuo I've deployed coordinator version "318271009812bf18ef7ef1785ecc9dffdbe7ee78-dirty-dmitshur-20190917T083031" just now with TLS 1.2, so we should be able to tell if it makes a difference with…
dmitshur commented on golang.org/x/build/cmd/coordinator: find inner modules when testing repos4d
(2 comments) Done. Agree. It'll be called once per go.mod file in the repository being tested. Compared to the time and cost of building the Go distribution from source and running repository tests…
dmitshur commented on golang.org/x/build/cmd/coordinator: factor out GOPATH dep fetching from runSubrepoTests4d
(1 comment) Testing confirmed that a typical error value here is something like "exit status 1". So when there are more than one, they show up as: Error: tests failed: exit status 1; exit statu…
dmitshur commented on github.com/gopherjs/gopherjs: support for being built and used with newer version of Go4d
It doesn't mean that. It means it'll be possible to use GopherJS 1.x while having Go 1.y installed. Go 1.13 support is tracked in #929.
dmitshur commented on golang.org/x/build/env/linux-mips: we have no Linux MIPS builders5d
I didn't get to this today, but I will try tomorrow morning. If we can't resolve the snapshot error on the coordinator side, then disabling it sounds reasonable. But I suspect it should be easy to f…
dmitshur commented on golang.org/x/build/cmd/gomote: warn when gomote binary is too old5d
Another possible instance of this problem is in https://github.com/golang/go/issues/33309#issuecomment-532001071 (see last paragraph of the comment). However, just fetching https://farmer.golang.o…
dmitshur commented on cmd/compile: race detector prints mis-leading stack trace5d
Ok, I think I see the problem in the code. **Update:** Read the last paragraph of this comment first. 1. Gomote "run" is called [here](https://github.com/golang/build/blob/1c6297c7b50cad1f0fad91b9…
dmitshur commented on cmd/compile: race detector prints mis-leading stack trace5d
Hmm. I have not used [racebuild](https://godoc.org/golang.org/x/build/cmd/racebuild), so I'm not sure how it works. `GO_DISABLE_OUTBOUND_NETWORK` may be relevant. It was added in March (see https:…
dmitshur commented on golang.org/x/build/cmd/coordinator: simplify fetchDependenciesToGOPATHWorkspace5d
> Is this mostly just renaming fetch to process? Yes, sorta. I'd recommend reading the final result: https://go.googlesource.com/build/+/refs/changes/78/195278/5/cmd/coordinator/coordinator.go#2540…
dmitshur commented on golang.org/x/build/cmd/coordinator: simplify fetchDependenciesToGOPATHWorkspace5d
(2 comments) This change goes along with the change above. Previously, it was: if !strings.HasPrefix(p, "golang.org/x/") || p == repoPath || strings.HasPrefix(p, repoPath+"/") { This C…
dmitshur commented on go/types: interfaces must be marked complete before they can be compared5d
Or move that line to a _test.go file, to make it a protobuf test failure rather than a protobuf build failure.
dmitshur commented on golang.org/x/build/cmd/coordinator: find inner modules when testing repos5d
Testing identified two problems: 1. It was testing the module at root twice unnecessarily 2. It was not skipping go.mod files inside "testdata" directories, "vendor" directories, or any directories …
dmitshur commented on golang.org/x/tools/go/packages: include more detail in internal error5d
This CL helped understand the problem, it's no longer needed.
dmitshur commented on cmd/go: 'go list -test' prints main package twice5d
> Unfortunately, I can't reproduce it locally yet. I figured out why it was failing to reproduce for me. I made the mistake of doing this: ``` PATH="$HOME/gotip/bin:$PATH" go version && go tes…