Activity

Yesterday
@andygrunwald It should be possible to infer the type from Gerrit's code. If [ProjectInput.java](https://cs.opensource.google/gerrit/gerrit/gerrit/+/master:java/com/google/gerrit/extensions/api/proje…
(10 comments) The #put-edit-file docs explicitly document 409 status code when "the change edit is a no-op". It doesn't seem this applies to #delete-edit-file, at least based on its doc…
(6 comments) Is a new field needed, given there's already a *http.Response? I expect e.Res.Request.URL (https://pkg.go.dev/net/http#Response.Request) can be used in HTTPError.Error instead. Si…
Thanks! Updates golang/go#28543. (I know it's somewhat orthogonal, but still nice to see in the timeline of that issue.) If you move the flag to be right under this comment, I think that'l…
(5 comments) I suspected this was an intentional desire to show up in a future scan for "does anyone use goto in non-generated Go code". 🙈 It seems the throwaway nit stole the highlight …
(1 comment) FWIW I was gonna delete the suggestion to use 'Fetch' since it doesn't fit well, but 'Find' or 'Lookup' might be better. Get's fine too.
+2, I think this can be submitted after unresolved threads are resolved. Since there may be changes that you'd want me to take another look before submitting, please feel feel to indicate so and…
(1 comment) != (Sorry, spotted after submission.)
(1 comment) I guess when we're using log.Fatal, we don't care the mutex happens not get unlocked. :)
(A quick +1 look with a trivial comment, leaving +2ing to the selected reviewer.) This part looks obsolete and can probably be removed (or updated, if you prefer).
(1 comment) Ah, yeah, it only needs to keep the connection open continuously, writing data continuously is optional. This way is shorter code and should be closer to what happens when remote command…
(1 comment) I believe it's still necessary to prevent a busy loop (the /exec handler needs to be writing data continuously). If you can suggest a simpler way to achieve that, it can be used.
This looks good to me (+1) but I think a x/pkgsite owner can review this further or comment if this shouldn't happen (e.g., because of future changes planned to this package). Jonathan, maybe you…
(1 comment) This was initially an "external" test: it just tested what happens after arbitrary timeouts in hope of catching a problem (without relying on implementation details of Exec too m…
(1 comment) When viewing https://proxy.golang.org in a browser, under "Status: Launched" heading it is mentioned: > These services are ready for production use. Please file issues if you…
Thanks for reporting. Since the field is optional, `MaxObjectSizeLimitInfo` is definitely not right as it'll always be included despite the `omitempty` tag, while `*MaxObjectSizeLimitInfo` would a…
This Week
dmitshur merged a change doc/go1.19: fix strconv.Quote pkg link1d
Sounds good, thanks. Having the flexibility to select multiple known issues will allow people to pick the path that works better. This might add an unnecessary empty line to the end of the message (…
Have you already considered but dismissed the idea of making the single known issue an umbrella issue, and link to childred builder issues from there? I'm asking because there's a bit more ov…
Sure. I see from logs there's now a GOPROXY set in the environment, and the 'i/o timeout' builder problem while downloading modules is resolved. Thank you!
CL 408434 had the effect that GOPROXY is no longer being set by coordinator. The latest slowbot failure shows it's still the value being used, perhaps because another value isn't provided in …
Thanks for working on it. Trying again with that change deployed.
I explored this possibility in September 2021 and wrote a short internal doc to report my findings. The summary was that this is feasible to do, but I found it was not worth doing in cmd/releasebot, …
(4 comments) Design looks fine to me, not seeing concerns. for creating a tag. Also s/TagInfo/TagInput/ at the start of the sentence. Illustrative value needs updating. Note that setting these v…
CC @ianlancetaylor.
dmitshur reviewed +2 on doc/go1.19: fix strconv.Quote pkg link3d
Thanks. Optionally, mention the tracking issue here: ``` For #51400. ```
Initial pass, leaving two high level comments and suggestions. Depending on your reply, will adjust second pass review accordingly. First, I think there's value in making code in the task packa…
Thank you.
Thanks. Consider adding here: For golang/go#46229. Since this change helps make progress on that issue.
(2 comments) If you'd like, remove "reflect." here as motivated in https://go-review.googlesource.com/c/go/+/401434/comments/1f7db341_ee8d4e06. For consistency, please keep the trailing…
(1 comment) Sure. If you'd like to more people to be listed as the builder owners (so we can reach them as needed), that is welcome and you can always take care of that in a follow up change. Th…
(1 comment) If you care, this is an instance of the longer host in a comment.
(Moving myself to CC since this has been reviewed.)
(+1 review since this already has +2.) Style note: when a function call that returns an error is inside an if statement like here and there's already an 'err' in parent scope, it can eit…
CL 407394 added a detailed error in case of a rare, unexpected problem. Propagate it in one more place so we benefit from the new information if this happens again. Updates golang/go#47695. Updates …
Thanks. If tests fail because gh panics, it means an addPerson entry with "@chenguoqi-cc" hasn't yet been added to the x/build/internal/gophers package.
(1 comment) I also noticed this is the first instance of a non-nil error being returned in the p.ForeachOpenCL iterator. It should either be logged and and return nil to keep going (to skip a bad/un…
dmitshur closed an issue cmd/go: workspace problems5d
dmitshur commented on cmd/go: workspace problems5d
Modules inside subdirectories indeed need to be tagged separately, as described at https://go.dev/ref/mod#module-path and https://go.dev/ref/mod#vcs-version. Since I understand there's no more issue …
Does the [`gl41core-cube`](https://github.com/go-gl/example/tree/master/gl41core-cube) example program run okay for you? If so, it has [similar code](https://github.com/go-gl/example/blob/d71b0d9f82…
Last Week
I've mailed [CL 407615](https://go.dev/cl/407615) that makes android-amd64-emu a post-submit builder only (in the main repo) while investigation of this issue is underway. If submitted, this issue ca…
Thanks for letting us know about this. I believe we're in agreement that this is okay. Approved.
dmitshur pushed to master in github.com/shurcooL/githubv41w
0b4e3294ff0076145153b32bdd09d1794ca0f387regenerate for schema changes by 2022-05-19
dmitshur pushed to master in github.com/shurcooL/graphql1w
bdb1221e171e552b0c7228c0837e6a2a2038b0f1ident: add GitLab, DevOps, IssueHunt, LFX brands
dmitshur commented on runtime: TestGcSys is still flaky1w
@mknyszek The test `TestGcSys` is still [checked in](https://cs.opensource.google/go/go/+/master:src/runtime/gc_test.go;l=23-33;drc=f01c20bf2ba889e5c9e3565175cc4276f9c11516) on tip, but with a skip t…
Thanks Ian. For the record, my upcoming coordinator change was going to include a 5 minute timeout on the x/build side via [`DistTestsExecTimeout`](https://pkg.go.dev/golang.org/x/build/dashboard#…
I suspect [CL 207908](https://go.dev/cl/207908) and [CL 208157](https://golang.org/cl/208157) made a lot of progress on this, and we're in a state where most buildlet methods that should have a conte…
I ran into this most recently while sending a trivial fix in https://go.dev/cl/406897, where a linux-amd64-longtest SlowBot request took almost 2 hrs instead of the usual 10-15 min, and while waiting…
I've observed a few more instances of this with `testsanitizers`. I think the root cause here is the same as issue #42699: some test is stalling, and eventually gets retried and succeeds. I've fil…
(This is mostly a continuation of #33249, with some new information.) There's evidence of a problem where this test can sometimes hang indefinitely, though in some environments there is an externa…
We generally have a global timeout for VM/pod-based builders (see #52929), though not sure about if reverse builders have something equivalent or not. I'll close this in favor of #35364, a more recen…
Running the test 100'000 times on a darwin/amd64 machine did not reproduce the problem. Running the same on a darwin/arm64 machine seems to reproduce after a while: ``` $ go test -count=100000…
`git-codereview` implements a fake Gerrit server (type `gerritServer`) and a git helper (type `gitTest`) for its tests, which might be relevant here (spotted via #50576).
Since it's not already mentioned, I'll add that one of the few ways to test `gopherbot` changes now is via its `-dry-run` mode. This helps, but it's not a complete and ideal solution. If we mock i…
My understanding of the state of this issue is that it can in Backlog milestone without a release-blocker label. That is, the known-failing test is skipped (via [CL 402181](https://go.dev/cl/402181))…
[CL 406355](https://go.dev/cl/406355) and [CL 406356](https://go.dev/cl/406356) updated golang.org/x dependencies. [CL 406359](https://go.dev/cl/406359) updated github.com/google/pprof. (Other indi…
dmitshur deleted branch in github.com/dmitshur/pprof1w
update-dep
dmitshur pushed to update-dep in github.com/dmitshur/pprof1w
There are newer versions of dependencies available. Start using them. I reviewed the change locally by comparing the diff of `go mod vendor` before and after. There were expected changes in x/sys/…
dmitshur pushed to update-dep in github.com/dmitshur/pprof1w
dmitshur created branch in github.com/dmitshur/pprof1w
update-dep
dmitshur deleted branch in github.com/dmitshur/pprof1w
update-dep
dmitshur pushed to update-dep in github.com/dmitshur/pprof1w
7d74b6bf53bff97dc3079980459a2c44318c1352all: update dependencies There are newer versions of dependencies available. Start using them.
dmitshur created branch in github.com/dmitshur/pprof1w
update-dep
[CL 406216](https://go.dev/cl/406216) increased the timeout to 2 hours, and longtest builds are completing on first try or so. Compare [before](https://user-images.githubusercontent.com/1924134/16840…
The comment of [`CleanUpOldVMs`](https://cs.opensource.google/go/x/build/+/master:internal/coordinator/pool/gce.go;l=648-661;drc=3ab5e7e87a80893ee88aba0e9de7e86cde9f5a0a) (and `CleanUpOldPodsLoop`) i…
dmitshur pushed to master in github.com/go-gl/glfw1w
eb3e265c76614051c36c141350d9740d439c068dall: fix typos in documentation
LGTM. Thanks.
dmitshur pushed to master in github.com/go-gl/glfw1w
8da84822faab099fbb16874b628a80a2b2a7ee9fv3.3/glfw: apply minor code cleanup
If you'd like, lines 174, 229, and 231 can also benefit from a similar fix.
Earlier
Yep, looks like it hasn't been added. Please feel free to send a PR if you have a need for it.
dmitshur pushed to master in github.com/go-gl/glfw1w
I think this is okay to add. Thanks.
Yeah, if we stop defining _GNU_SOURCE and that causes a warning in GLFW C codebase due to _GNU_SOURCE not being defined, that might be something that is better suited to be addressed upstream. Are…
dmitshur pushed to master in github.com/go-gl/glfw1w
597ae79a86c6cf2997fc178267cff9a519f851c9v3.3/glfw: update to GLFW 3.3.7
To clarify, are you running into this when using a .wasm module built with Go 1.18 together with the wasm_exec.js file from Go 1.18? Are you able to reproduce it with a simple hello-world program, or…
There are definitely regular updates being printed now which provide pretty good information, so I think this issue is less actionable now. We can consider this to be resolved by [CL 207841](https://…
In issue #52856 it was determined that the current 1.18 behavior is correct, so no fix needs to be backported to 1.18.
To me, the warning that `_GNU_SOURCE` is defined twice seems more harmless. If you're not able to find a solution for `ppoll` problem easily, I'd suggest let's revert 692d67bc69fe52c59d22b0fcad6adff6…
A possibly minified repro (with `struct { int }`): https://go.dev/play/p/F_PIyGVxd0E.
Adding a data point observed in Git for Windows release notes [upcoming breaking changes](https://github.com/git-for-windows/build-extra/blob/76d6b0286c275eb445af2f4ab2fae41fe302341c/ReleaseNotes.md#…
Fixed for Go 1.19 in [CL 399539](https://go.dev/go/399539). (This didn't get closed because its commit message had "For" rather than "Fixes".)
There doesn't appear to be anything abnormal for yesterday at https://console.cloud.google.com/appengine?project=golang-org nor https://console.cloud.google.com/monitoring/uptime?project=golang-org (…