Activity
Today
dmitshur reviewed +2 on golang.org/x/talks/app.yaml: remove GOLANGORG_CHECK_COUNTRY2h
(1 comment)
After the inline comment is resolved, this change LGTM.dmitshur merged a change README: [release-branch.go1.16] pull gopher image from website2h
dmitshur reviewed +2 on golang.org/x/build/cmd/tip: stop using golangorg -index and -play3h
(1 comment)
This may need to be updated too, if the /search endpoint is removed.dmitshur reviewed +2 on golang.org/x/build/cmd/tip: stop using golangorg -index3h
(1 comment)
Fixes golang/go#44613.dmitshur opened an issue golang.org/x/build/cmd/tip: fails to serve website at CL 293412 or newer7h
https://farmer.golang.org/#health reports a problem with the tip.golang.org website.
[CL 293412](https://golang.org) removed the -index flag. `cmd/tip` needs to be updated accordingly. Otherwise i…dmitshur merged a change golang.org/x/website/_content/doc: highlight 'remove old Go tree' step before extracting7h
dmitshur commented on golang.org/x/talks/app.yaml: remove GOLANGORG_CHECK_COUNTRY8h
(1 comment)
This does not get a version of x/tools that includes CL 293449.dmitshur reviewed +1 on golang.org/x/blog/chore: to avoid duplicate names on example8h
(1 comment)
Please rename the PR title to:
content/context-and-structs.article: fix duplicate arguments
See https://golang.org/wiki/CommitMessage.Yesterday
dmitshur commented on doc/go1.16: add code tags to nil21h
(1 comment)
Thanks for sending the change.
I considered it before the release, but found that even after this change there's one more instance still using nil without being wrapped in <code>. When …dmitshur commented on golang.org/x/tour/app.yaml: remove GOLANGORG_CHECK_COUNTRY21h
(1 comment)
Can you update this to require a version of x/tools that includes CL 293449?
(Same comment for the equivalent blog and talks CLs.)dmitshur reviewed +2 on golang.org/x/tools/playground: remove /share registration, add Proxy21h
(4 comments)
This change LGTM.
Given that the functionality this package provides is so trivial, but the overhead caused by having an indirect abstraction is relatively constant, I think things wou…dmitshur closed an issue golang.org/x/build/dashboard: remove darwin-386-10_14 builder when Go 1.14 stops being supported1d
dmitshur commented on golang.org/x/build/dashboard: remove darwin-386-10_14 builder when Go 1.14 stops being supported1d
Resolved in [CL 293771](https://golang.org/cl/293771).dmitshur commented on golang.org/x/build: use new name for release branches in golang.org/x repos starting with Go 1.161d
We've updated our release process to start creating branches that follow the new internal naming scheme, and it has been used with the Go 1.16 release.
We've also updated permissions in [CL 283262…dmitshur closed an issue golang.org/x/build: use new name for release branches in golang.org/x repos starting with Go 1.161d
This Week
dmitshur reviewed +2 on golang.org/x/playground/all: support Go 1.16 in the Go Playground2d
(8 comments)
Typo here.
s/go/Go/
As of this CL, this image now contains both a normal and faketime Go installation, and what they have in common is they're both built at GO_VERSION version.
Optio…dmitshur merged a change doc: start draft go1.17 release notes, move go1.16 to x/website2d
dmitshur merged a change golang.org/x/website/_content/doc: add Go 1.16 release notes2d
dmitshur commented on golang.org/x/website/_content/doc: add Go 1.16 release notes2d
(1 comment)
Done.dmitshur merged a change golang.org/x/build/cmd/gerritbot: check for nil *github.Response before accessing2d
dmitshur reviewed +2 on golang.org/x/playground/sandbox: always use latest runsc version2d
dmitshur starred in github.com/evanw/esbuild2d
dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: clean up, centralize handler registration2d
(2 comments)
Oh, indeed. I see how I got confused:
I initially tested it on the Go 1.16 blog post like https://golang.org/blog/go1.16, which didn’t work, but that looks like a bug due to it havin…dmitshur commented on golang.org/x/build: misc-compile TryBots are too slow3d
I too have observed this:
<img src="https://user-images.githubusercontent.com/1924134/108784909-5f2ae180-753e-11eb-88e6-f93311e9c182.png" width=303px>
When these trybots take the longest, they …dmitshur opened a change golang.org/x/website/_content/doc: add Go 1.16 release notes3d
Copied from Go commit 094048b93845c08e0f2db8639764f9564e64835b,
deleted there in CL 295209.dmitshur commented on all: reopen tree for Go 1.17 development3d
The "Current Tree Status" was updated from "Open for bug fixes only" to "Open for Go 1.17 development". I've sent an announcement to [golang-dev](https://groups.google.com/g/golang-dev/c/VNJFUxHWLHo/…dmitshur closed an issue all: reopen tree for Go 1.17 development3d
dmitshur opened a change doc: start draft go1.17 release notes, move go1.16 to x/website3d
This template is based on CL 248198 and previous ones like it.
Continue to eagerly include often-used sections, and clarify that
the TODO is about completing the section, or removing if it turns
out …dmitshur opened an issue doc: write Go 1.17 release notes3d
Tracking bug for writing the Go 1.17 Release Notes.
The latest state on tip can be viewed at https://tip.golang.org/doc/go1.17 (the page doesn't exist yet, so it's currently 404; that'll be fixed …dmitshur commented on cmd/internal/moddeps: confusing error message when difference is due to a "replace" directive3d
Can this be fixed by changing `t.Errorf("cannot check consistency for filesystem-local replacement in module ...` into `t.Fatalf`? It seems the rest of the test doesn't produce a helpful output if th…dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: clean up directory listings3d
(1 comment)
Phew, done reviewing this stack (first pass). Please me know when you're ready for me to take another look at what's still left.dmitshur reviewed +2 on golang.org/x/website/internal/godoc: delete /cmd and /pkg serving code3d
(9 comments)
I think these are all gone except "text", which isn't included here.
No longer does. Also s/godoc/golangorg/ if you choose to include this file in this CL.
Also gone.
These are gone.…dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: redirect /cmd and /pkg to pkg.go.dev3d
(3 comments)
Need to add a "golang/go" prefix since this isn't the main repo:
Fixes golang/go#44356.
These lines can be updated to match the new behavior.
The current redirect code handles im…dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: move googleCN code out of internal/godoc3d
(2 comments)
This is needed in prod mode, but less sure whether it's needed in local mode. I guess having it in both modes is fine for now.
Consider making it more clear that it's okay for this to …dmitshur reviewed +2 on golang.org/x/website/internal/godoc/util: merge into internal/godoc3d
dmitshur reviewed +2 on golang.org/x/website/internal/godoc: remove vfs3d
(1 comment)
🎉dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: switch from vfs to io/fs3d
(3 comments)
The commit message doesn't say, but I understand we consider gatefs no longer needed (and will follow up if it turns out otherwise).
Please document bindFS and its behavior.
It should…dmitshur reviewed +2 on golang.org/x/website/internal/godoc: delete RootType3d
(1 comment)
Deleting RootType SGTM. I'm not sure if I understand whether the replacement is sound, but I know all this code is being deleted in a future CL in this stack, so I skipped reviewing that…dmitshur reviewed +2 on golang.org/x/website/internal/godoc: delete indexing code3d
dmitshur commented on golang.org/x/website/internal/godoc/golangorgenv: delete3d
(3 comments)
Note: we can delete the internal/godoc/golangorgenv package even without landing CL 293418 and CL 293419. Everything in x/website can use internal/env.
But I'll wait to see your replie…dmitshur reviewed +2 on golang.org/x/website/internal/redirect: add minor fixes from golang.org/x/tools/godoc/redirect3d
dmitshur reviewed +2 on golang.org/x/website/internal/godoc/redirect: remove unused package3d
dmitshur reviewed +2 on golang.org/x/website/internal/godoc: remove analysis3d
(2 comments)
As an aside: it's unfortunate in that we could have—it would've been fast to have it enabled since golang.org used to serve only the standard library packages (analysis mode would get…dmitshur reviewed +2 on golang.org/x/website/internal/godoc/static: delete unused package3d
dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: change imports to use internal/godoc3d
(2 comments)
These changes aren't correct, looks like a bad "replace all", please undo.
This change may actually be okay, but that package is being deleted in the next CL in the stack, and this REA…dmitshur reviewed +2 on golang.org/x/website/internal/godoc: fork golang.org/x/tools/godoc@123adc3d
(1 comment)
I worry a 6 character hash may stop being uniquely identifying within Go's lifetime with a high probability. Can you please replace it a full length one also add the CL number:
This i…dmitshur commented on golang.org/x/website/cmd/golangorg: add China playground restriction3d
(3 comments)
See my comment on CL 293449. There are some concerns here and I think we can skip this for now, revisit it after.
This function is unused. I understand it was meant to be assigned to p…dmitshur commented on golang.org/x/website/go.mod: update golang.org/x/tools3d
(1 comment)
See my comment on CL 293449. There are some concerns here and I think we can skip this for now, revisit it after.dmitshur commented on golang.org/x/tools/playground: remove Google-specific service check3d
(2 comments)
IIUC the motivation for this CL is to simplify things given the x/website stack ending at CL 293495. However this package is used in more places than just x/website, so I'd suggest let'…dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: fix various comments3d
dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: clean up, centralize handler registration3d
(3 comments)
Question. We haven't supported URLs like https://golang.org/blog/go116-module-changes in the past, only https://golang.org/blog. Is it intentional they're added? What's the benefit comp…dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: make /x redirect message nicer3d
(1 comment)
This period can be removed since it's preceded by an ellipsis.dmitshur reviewed +2 on golang.org/x/website/_content: create explicit lib/godoc tree3d
dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: simplify local vs prod programs3d
(5 comments)
Remove " && !prod"; this file is meant to be shared between local and prod modes.
(This will fix build errors like "undefined: httpAddr".)
Were these constraints moved on top of the c…dmitshur reviewed +2 on golang.org/x/website/cmd/golangorg: delete index support code3d
(3 comments)
Add:
For golang/go#41102.
Since this change also removes the -zip flag (item 3 in that list).
(FYI I've reviewed up to CL 293489 in this stack so far; will resume after a meeting…dmitshur commented on golang.org/x/build/all: go fmt ./...3d
(2 comments)
I see, thanks for the explanation.
Thanks for the explanation (I see no manual editing is ever required: I can use gotip fmt instead).
> If you are using Go built from master (I hope …dmitshur commented on golang.org/x/website: show search box on smaller viewports3d
Indeed, this isn't relevant by now, closing.dmitshur closed an issue golang.org/x/website: show search box on smaller viewports3d
dmitshur closed an issue golang.org/x/website: remove search box from golang.org3d
dmitshur commented on golang.org/x/website: remove search box from golang.org3d
Fixed by [CL 292590](https://golang.org/cl/292590), closing. (It didn't get closed automatically because the "golang/go" prefix wasn't included.)
@magical The commit message suggests that the migr…Last Week
dmitshur starred in github.com/aarzilli/gdlv5d
dmitshur commented on golang.org/x/build/all: go fmt ./...5d
(1 comment)
If this isn't needed for #44222 (since no package from x/build is vendored into the main repo), I'd prefer we submit this after Go 1.17 is out, to avoid adding manual maintenance overhea…dmitshur commented on golang.org/x/build/all: go fmt ./...5d
(1 comment)
If we need to change the build constraints before Go 1.17 is released, will we need to update the //go:build line by hand? As far as I understand, gofmt won't do it for another 6~ months.dmitshur commented on golang.org/x/website/fix: mac osx min version6d
(1 comment)
The previous value seems correct to me.
It's there to match very old macOS release files, like "go1.4.darwin-386-osx10.8.pkg", that had a "-osx10.8" suffix. See https://golang.org/dl/#g…dmitshur opened a change golang.org/x/build/cmd/gerritbot: check for nil *github.Response before accessing6d
When github.Client.Do returns a non-nil error, the *github.Response
may be nil if a request wasn't even made. Check for that to avoid a
panic.
Fixes golang/go#44438.dmitshur opened an issue golang.org/x/build/cmd/gerritbot: possible panic in getFullPR6d
Observed this panic from 43 days ago:
```
2021/02/04 06:15:49 Retrieving PR golang/blog#34 from GitHub using Etag "W/\"8a7ebf3c8163c89f842d22ec8a135ec89f9af0061f034bbd6a13ad716abff05b\"" ...
pan…dmitshur reviewed +2 on runtime/pprof: disable TestMorestack on darwin/arm646d
(1 comment)
Is it okay there's no tracking issue being opened for this?dmitshur commented on golang.org/x/build/cmd/gopherbot: may exceed GitHub API rate limit6d
Based on https://github.com/golang/crypto/pull/143#issuecomment-782377386, this issue might be affecting more services than just GopherBot.dmitshur commented on golang.org/x/crypto/autocert: Include the domain in the SAN of the CSR6d
(2 comments)
The prefix should be "acme/autocert:".
Lower case 'include' after.
Please wrap these lines, and move the fixes line separately:
More compliant ...
with Pebble ...
Fixes …dmitshur commented on golang.org/x/crypto/autocert: Include the domain in the SAN of the CSR6d
It should work now. But, from GerritBot logs:
```
2021/02/19 21:40:32 GitHub: 0/5000 calls remaining; Reset in 7m52.975721092s
```
It likely shares rate limit with GopherBot, so issue #44406 …dmitshur commented on golang.org/x/crypto/autocert: Include the domain in the SAN of the CSR6d
@FiloSottile [GerritBot](http://golang.org/wiki/GerritBot) waits for @googlebot to add a "cla: yes" label before starting work. For some reason that hasn't happened here yet.dmitshur opened a change golang.org/x/website/_content/doc: highlight 'remove old Go tree' step before extracting6d
Fixes golang/go#44426.dmitshur commented on golang.org/x/blog/x/blog/content: add contexts and structs article6d
(1 comment)
Please remove the "x/blog/" prefix, leaving just "content:".
That prefix is needed in Go issues, because all repositories share the same issue tracker. It's not needed in commit subject…dmitshur commented on golang.org/x/build: Add FreeBSD 11.4 and 12.2 builders6d
CC @golang/release.dmitshur merged a change golang.org/x/build/dashboard: remove what's unused as of Go 1.16 release6d
dmitshur merged a change golang.org/x/build/cmd/releasebot: add check for "create first minor release milestone" step6d
dmitshur merged a change golang.org/x/build/dashboard: mark windows-arm64 as known missing builder6d
dmitshur reviewed +2 on golang.org/x/build/cmd/buildlet: add support for new macOS versioning6d
(2 comments)
Minor: Consider mj >= 11 so that this number visually matches the "As of macOS *11*.0 (Big Sur), ..." comment on top.
If you prefer leaving it as is, that's good too.
Checking my unde…dmitshur commented on golang.org/x/playground: 1.16 not available6d
Thanks for reporting.
CC @golang/release.dmitshur commented on golang.org/x/build: freebsd-amd64-race builder should be updated to use a newer FreeBSD version6d
For now I'll resolve this specific issue by updating freebsd-amd64-race to use the available FreeBSD version, 12.0.
Please file a separate issue about adding new builders that target newer version…dmitshur closed an issue golang.org/x/build: freebsd-amd64-10_3 trybot took over 20 minutes6d
dmitshur commented on golang.org/x/build: freebsd-amd64-10_3 trybot took over 20 minutes6d
The `freebsd-amd64-10_3` builder is long gone (removed in [CL 246639](https://golang.org/cl/246639)), so this specific issue can be closed. (The general tracking issue for TryBot speed is #17104.)dmitshur opened a change golang.org/x/build/dashboard: mark windows-arm64 as known missing builder6d
The Go 1.17 tree has started adding the windows/arm64 port.dmitshur opened a change golang.org/x/build/cmd/releasebot: add check for "create first minor release milestone" step6d
The release process now documents the step of creating the first minor
release milestone when issuing a major Go release. That happens before
the 'releasebot -mode=prepare' step.
Add an automated ch…dmitshur commented on golang.org/x/website: webside 404 issue6d
Thanks, we'll deploy today. @toothrot is already on it, please close this issue when done.dmitshur commented on golang.org/x/website: #go-install anchor in mod reference doesn't exist but is used in a few places6d
Yes, I'm doing a deploy now (also for #44414).dmitshur commented on : GO 1.16 Program do not compile on raspberry pi6d
Thank you for the feedback. I've filed #44426 about improving the Linux installation instructions.dmitshur commented on golang.org/x/website: highlight removal of previous Go install in Linux installation instructions6d
Given how important the step of removing the previous /usr/local/go directory before extracting, I think we must include it in the numbered steps.
The current instructions link to https://golang.o…dmitshur opened an issue golang.org/x/website: highlight removal of previous Go install in Linux installation instructions6d
Experience and feedback in #44320 (thank you @jpmieville, @davecheney, @agnivade) suggests that the current Linux installation instructions are prone to a user mistake: missing the step of removing t…dmitshur commented on all: reopen tree for Go 1.17 development6d
@thanm Yes, as Russ said that CL is a follow up to the dev.regabi merge and is okay. Thanks.dmitshur commented on README: [release-branch.go1.16] pull gopher image from website6d
(1 comment)
Thanks for reviewing. I'll submit this if/when #44402 is approved for backport.dmitshur opened an issue golang.org/x/build/cmd/gopherbot: GopherBot may exceed rate limit6d
While investigating x, I noticed in GopherBot's logs that it was failing to take some actions due to exceeding GitHub API rate limit quota:
```
$ kubectl logs -f gopherbot-deployment-6c6d86d5b9-s…dmitshur commented on golang.org/x/build/cmd/gopherbot: mark Gerrit comments as CI-generated6d
This may have been resolved by placing the Go Bot account (and Gerrit Bot) in a "Service Users" group for Gerrit's Attention Set feature (documented [here](https://gerrit-review.googlesource.com/Docu…dmitshur opened an issue golang.org/x/build/cmd/releasebot: should also create the next minor release milestone6d
I noticed that GopherBot hadn't closed #44346 even though [CL 293411](https://golang.org/cl/293411) was submitted. I figured out the problem by looking at its logs and seeing:
```
2021/02/19 00:2…dmitshur opened a change README: [release-branch.go1.16] pull gopher image from website1w
Fixes breakage accidentally introduced by https://golang.org/cl/291711.
Fixes #44402.
Updates #44295.dmitshur commented on doc: Broken image in readme1w
@gopherbot Please backport this to Go 1.16. It is a safe documentation fix needed to restore the gopher image at https://pkg.go.dev/std#section-readme (and any other place that displays the README of…dmitshur commented on access: add zpavlinovic to go-assignees and go-approvers1w
CC @golang/release.dmitshur commented on golang.org/x/tools/internal/imports: update stdlib index after every major Go release1w
@heschik Yep, that makes sense. The reason I've used this early-in-cycle path for now is the same reason I closed https://github.com/golang/go/issues/27155#issuecomment-779940755: we currently don't …dmitshur commented on golang.org/x/tools/internal/imports: automatically update stdlib index every major release1w
As a first step, let's make this a recurring early-in-cycle release-blocking issue, similar to #40705 and #36905, so that this task isn't missed.
I'll place it in Go 1.17 milestone now, and when […