Activity
Today
dmitshur commented on golang.org/x/website/solutions: update urfave/cli link to v3 GitHub URL5h
@Jah-yee Thanks for sending this change. Please see information posted in https://github.com/golang/website/pull/359#issuecomment-4317356043. I left review comments at https://go.dev/cl/770646.dmitshur commented on golang.org/x/website/solutions: update urfave/cli link to v3 GitHub URL5h
Thanks for updating this entry. See inline comments for a few suggestions.
Moved the 'Fixes' line to the bottom of the commit message. Done.
Maybe it's worth it to include "v3" here too, since this…Yesterday
dmitshur commented on encoding/json: consider implications of using TextAppender in V1 backwards-compatibility mode22h
That mentioned package has a comment in its AppendText method implementation:
```
// NOTE(anderseknert): this DOES allocate still, and while that's unfortunate,
// we'll be better off dealing with t…dmitshur reviewed +1 on cmd/go/internal/doc: clear packageCache before building pkgsite1d
This seems reasonable to me, but I defer to Michael for +2 review. I left minor suggestions in inline comments. Thanks.
It'd be good to document what this exported function does, and maybe mention t…dmitshur reviewed +2 on cmd/go/internal/doc: update pkgsite version1d
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: remove unnecessary TODOs1d
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27: remove TODOs for subrepo changes1d
Thanks.dmitshur opened a change golang.org/x/build/cmd/updatestd: maintain cmd/go/internal/doc selection of pkgsite version1d
Make it easier to update the version included as a constant inside
the cmd/go/internal/doc command by having updatestd take it on too.
For golang/go#36905.dmitshur opened a change cmd/go/internal/doc: move buildPkgsite version constant to package scope1d
This will make it easier to have its value maintained as part of the
x/build/cmd/updatestd command used to help with the go.dev/issue/36905
dependency maintenance process.
For golang/go#36905.dmitshur commented on golang.org/x/pkgsite/cmd/internal/doc: update dependencies1d
(1 comment)
This update can count as the "soon after the release freeze starts" update that we normally do as part of issue #36905. We're still before Go 1.27 RC 1.
We may want to consider starting…dmitshur reviewed +2 on golang.org/x/pkgsite/cmd/internal/doc: update dependencies1d
Thanks.dmitshur reviewed +2 on golang.org/x/build/cmd/relnote: added markdown summary info from issue and CLs for each TODO1d
(3 comments)
```suggestion
provenance: fmt.Sprintf("RELNOTE comment in [CL %d](/cl/%[1]d)", cl.Number),
```
```suggestion
provenance: fmt.Sprintf("[CL %d](/cl/%[1]d)", cl.Number),
```
The p…This Week
dmitshur commented on golang.org/x/tools/go/analysis/passes/stdversion: language vs toolchain version2d
I'm not entirely sure where the best documentation for per-file language version is, so I'll refer to https://go.dev/doc/toolchain#config as the best I could find so far. It says:
> The go line for …dmitshur commented on golang.org/x/build/main.star: remove openbsd-arm64 from KNOWN_ISSUE_BUILDER_TYPES2d
Thanks. This might need to be rebased and regenerated if luciconfig.TestValidate fails.
Some time has passed and results are visible:
https://ci.chromium.org/ui/p/golang/g/port-openbsd-arm64/builde…dmitshur commented on cmd/go: `-help` and `go version` are broken when go.mod contains non-standard version2d
As far as I can tell, the original reproducer works with go1.21rc2 (as the local toolchain), and not with go1.21rc3. Plausibly fixed by [CL 509097](https://go.dev/cl/509097). Setting the milestone to…dmitshur closed an issue github.com/shurcooL/vfsgen: Test issue2d
dmitshur commented on crypto/tls: benchmark needs no entropy, old suites3d
Thanks. This has two +2s and is needed to fix longtest-race builders, so letting this submit sooner. If there's more to do beyond what CL 779984 takes on, it's worth filing an issue (it can be releas…dmitshur commented on cmd/link: TestELFHeadersSorted/exe failures3d
CC @golang/riscv64.dmitshur commented on golang.org/x/tools/gopls: crash in runtime trace writer3d
From issue triage, this may be explainable by memory corruption.dmitshur commented on runtime:cpu4: TestCgoCallbackGC failures3d
From release triage, this is probably consistent with a timeout at the package-level.dmitshur commented on runtime: TestFakeTime failures3d
From release triage, this also could be a timeout at the package-level.dmitshur closed an issue runtime: TestSecretCgo failures3d
dmitshur commented on runtime:cpu1: TestSecretCgo failures3d
From release triage, these could be timeouts at the package-level.dmitshur reopened an issue cmd/fix: modernizer for direct references to embedded fields in struct literals3d
dmitshur commented on golang.org/x/build/internal/installer/darwinpkg: remove "Fixing permissions" step3d
Thanks.dmitshur reviewed +2 on cmd/go: [release-branch.go1.25] use fsys.ReadDir for IsStandardPackage3d
Thanks.dmitshur reviewed +2 on cmd/go: [release-branch.go1.26] use fsys.ReadDir for IsStandardPackage3d
Thanks.dmitshur reopened an issue golang.org/x/tools/gopls: modernizer for direct references to embedded fields in struct literals3d
I understand the intent of https://github.com/golang/go/issues/77965#event-26295992291 is to make sure this change makes its way into cmd/fix, so this still needs to be updated using the process desc…dmitshur commented on cmd/fix: modernizer for direct references to embedded fields in struct literals3d
I understand the intent of https://github.com/golang/go/issues/77965#event-26295992291 is to make sure this change makes its way into cmd/fix in time for Go 1.27 RC 1, so this still needs to be updat…dmitshur commented on encoding/json/v2: new API for encoding/json3d
This is currently blocking Go 1.27 release candidate 1. I understand all that's left to close this is adding release notes.dmitshur fixed an issue simd: support WASM and limited machine-independent SIMD under a GOEXPERIMENT [freeze exception]3d
The originally requested freeze exception was completed, closing.dmitshur commented on simd: support WASM and limited machine-independent SIMD under a GOEXPERIMENT [freeze exception]3d
The originally requested freeze exception was completed, closing.dmitshur commented on golang.org/x/build/...: use 'an error satisfying errors.Is/As(err, target)' phrasing3d
(1 comment)
This change is ready for review.
Thanks.dmitshur commented on golang.org/x/vuln/cmd/govulncheck: fix exit code propagation for wrapped scan errors3d
Thanks for working on a fix. I may not be the best reviewer for this since I'm not familiar with x/vuln and conventions or constraints involved here. I left some questions, but if there's someone bet…dmitshur commented on golang.org/x/build/...: use 'an error satisfying errors.Is/As(err, target)' phrasing3d
Friendly ping since it's been a while.dmitshur reviewed +2 on golang.org/x/website/_content/doc: remove uneeded TODOs4d
Thanks.dmitshur reviewed +2 on golang.org/x/website/internal/history: document Go 1.26.4 and 1.25.11 releases4d
Thanks. I'm not seeing any issues.dmitshur reviewed +2 on golang.org/x/website/_content/doc: add backwards compatibility note for ppc64/linux4d
Thanks.dmitshur reviewed +2 on golang.org/x/website/_content/doc: remove unneeded release notes TODOs4d
Thanks.dmitshur reviewed +1 on golang.org/x/website/internal/history: document Go 1.26.4 and 1.25.11 releases4d
+1 so David can review more closely.dmitshur commented on golang.org/x/build/main.star: add a linux-amd64-racegocommand-noopt builder at tip4d
Thanks.dmitshur opened an issue encoding/json/v2: keep v2 and jsontext packages buildable under GOEXPERIMENT=nojsonv24d
In Go 1.26 and older, GOEXPERIMENT=jsonv2 had the effect of "make still-experimental encoding/json/v2, encoding/json/jsontext packages available, and also modify the encoding/json implementation to b…dmitshur reviewed +2 on golang.org/x/website/_content/doc: add backwards compatibility note for ppc64/linux4d
Thanks.
Yes, it's very helpful. You can include "For golang/go#78779." in the commit message because that's the tracking issue for doing this. Thanks.
https://go.dev/wiki/Spelling suggests avoiding…dmitshur commented on golang.org/x/mobile/internal/sdkpath: accept minor Android platform versions4d
There are some notes relevant to this in [this golang-dev thread](https://groups.google.com/g/golang-dev/c/4Li4Ovd_ehE).dmitshur commented on golang.org/x/build: try a -racegocommand builder4d
Sent [crrev.com/c/7892066](https://crrev.com/c/7892066) and [go.dev/cl/785920](https://go.dev/cl/785920).dmitshur opened a change golang.org/x/build/main.star: add a linux-amd64-racegocommand-noopt builder at tip4d
Add a post-submit-only builder that runs at tip with a new racegocommand
mod, one which will reinstall the go command with race mode enabled.
Also try combining it with a noopt mod, since the existin…dmitshur opened an issue golang.org/x/build: try a -racegocommand builder4d
Between #77053, #79667, #79680, there may be a remaining data race in the `cmd/go/internal/load.PackagesAndErrors` implementation. Maybe a builder similar to [-racecompile](https://cs.opensource.goog…dmitshur commented on net/http/httptest: synctest support5d
@neild [CL 769521](https://go.dev/cl/769521) is submitted and there's a [release note entry](https://tip.golang.org/doc/go1.27#nethttphttptestpkgnethttphttptest). Is there anything more to do for Go …Last Week
dmitshur commented on internal/buildcfg: [dev.simd] turn SIMD goexperiment back on1w
(1 comment)
Sounds good, thanks.dmitshur reviewed +2 on golang.org/x/build/...: fix some comments to improve readability1w
Thanks.dmitshur commented on internal/buildcfg: [dev.simd] turn SIMD goexperiment back on1w
(1 comment)
The cmd/api:check.TestCheck failure here is due to CL 784980.
You can consider either starting to add the (admittedly large) API files to a doc/next/nnn.txt file, as that would make the…dmitshur commented on : policy for removing GODEBUG flags1w
The proposal process to define a policy for removing GODEBUG flags has considered many nuances and reached the likely accept state. I did a quick grep to see if GOEXPERIMENT came up here, and it hasn…dmitshur opened an issue internal/goexperiment: delete Flags.GenericMethods field in time for Go 1.27.01w
The discussions that I recall seeing, as well as the commit messages of [CL 772441](https://go.dev/cl/772441), [CL 779221](https://go.dev/cl/779221) and [CL 779301](https://go.dev/cl/779301), and htt…dmitshur commented on golang.org/x/website/_content/doc: document new `go mod tidy` behavior1w
(1 comment)
Ah, thanks. No need, either having the blank lines or not having them is fine.dmitshur commented on api: promote next to go1.271w
For posterity, that link should've been #79737.dmitshur commented on golang.org/x/website/_content/doc: document new `go mod tidy` behavior1w
(2 comments)
(a minor suggestion for the future)
For a release notes CL, the [motivation](https://go.dev/wiki/CommitMessage#non-normative-references) is mostly clear, so a blank commit message body…dmitshur reviewed +2 on golang.org/x/website/_content/doc/go1.27.md: tweak release notes1w
Thanks.
"new ValueEnd field" implies the field is newly added in Go 1.27, but it was actually added back in Go 1.26:
https://go.dev/doc/go1.26#goastpkggoast:~:text=The%20new%20BasicLit.ValueEnd%20f…dmitshur commented on cmd/api: include baseline goexperiment.* tool tags1w
(1 comment)
I agree there's no reason to do it. The point I was making is that if it were to happen accidentally and no one noticed, we could issue a major Go release with an exported API without th…dmitshur opened an issue cmd/api: doesn't set GOARCH-specific ToolTags and ReleaseTags in the build context, may have false negatives or positives compared to actual API1w
While working on a fox for issue #79737, it became apparent that there's some skew between the build context used by the go command when building standard library packages, and the build context used…dmitshur commented on api: audit for Go 1.271w
The missed APIs have been added, and the API list can proceed with the audit.dmitshur commented on cmd/api: include baseline goexperiment.* tool tags1w
Thanks.dmitshur commented on cmd/api: include baseline goexperiment.* tool tags1w
(1 comment)
Yeah, my first attempt to fix this was trying to make the API check start with build.Default. That actually didn't work because of the GOARCH-specific constraints like arm64.v8.0 (and th…dmitshur opened a change cmd/api: include baseline goexperiment.* tool tags1w
There are some exported package APIs that have started out as being
gated behind a GOEXPERIMENT, requiring users to explicitly opt in,
and have since become enabled for all users. This was done by ha…dmitshur commented on cmd/go: start a test case for an insecure scheme check1w
(2 comments)
I mistakenly thought this was needed (surprisingly to me), but Michael hinted that it's not. (I might've been needed before I added a go.mod file.) Removed in PS 2.
Done in PS 2.dmitshur reviewed +2 on golang.org/x/build/main.star: enable x repos on openbsd-arm64 builder1w
dmitshur reviewed +2 on golang.org/x/build/main.star: remove openbsd-arm64 from KNOWN_ISSUE_BUILDER_TYPES1w
Thanks.
Let's give CL 784840 a chance to get submitted first and see how the builder runs on x/ repos. If it's generally in a good shape, then we can proceed with submitting this too. A few repo-spe…dmitshur commented on golang.org/x/build/cmd/relnote: added markdown summary info from issue and CLs for each TODO1w
(1 comment)
Hmm, I should've put that code in a block that so the Markdown string doesn't render in the Gerrit comment view. 😅
```
fmt.Sprintf("TODO: accepted [proposal %d](/issue/%[1]d)", num)
…dmitshur commented on golang.org/x/build/cmd/relnote: added markdown summary info from issue and CLs for each TODO1w
Thanks for doing this. A couple suggestions I was thinking about, since you're already planning to improve this to handle link generation better. I'll review after that.
I'm not sure I understand th…dmitshur reviewed +2 on golang.org/x/website/_content/doc: make TODO cl and issue links in go1.27.md work1w
Thanks. I was going to leave this suggestion on relnote tool CL too.dmitshur commented on cmd/go: [release-branch.go1.26] use fsys.ReadDir for IsStandardPackage1w
(1 comment)
Not (yet) applied here.dmitshur commented on cmd/go: [release-branch.go1.25] use fsys.ReadDir for IsStandardPackage1w
Trybot failures need attention.dmitshur commented on cmd/go: [release-branch.go1.26] use fsys.ReadDir for IsStandardPackage1w
The trybot failures seem related to the backport. Please add this back to my attention set once that's resolved.
```suggestion
Fixes #79721
```dmitshur reviewed +2 on reflect: linkify the docs for Value.Seq and Value.Seq21w
Thanks.
FWIW, I tried looking a bit into how doc comment linking can be used when brackets are involved, and found it may be viable to do some linking.
That is, it is possible, though it does mean …dmitshur reviewed +2 on golang.org/x/build/main.star: enable x repos on openbsd-arm64 builder1w
Thanks!dmitshur commented on api: promote next to go1.271w
Filed #79733 for the problem in cmd/api.dmitshur commented on api: audit for Go 1.271w
Thanks for catching that. Filed #79737 for cmd/api.dmitshur opened an issue cmd/api: misses encoding/json/v2 API changes1w
@prattmic noticed and reported in https://github.com/golang/go/issues/79733#issuecomment-4575506443 that the cmd/api tool missed encoding/json/v2 API changes. It may be related to the API being gated…dmitshur commented on cmd/go: document new `go mod tidy` behavior1w
Thanks for preparing this release note.
The doc/next files in this repo were deleted in CL 784700. The process of filing the API audit issue (#79733) and merging the release note fragments into x/we…dmitshur commented on golang.org/x/build/internal/relui: only run TestRelease/major on 1.26 and newer1w
Thanks.dmitshur commented on golang.org/x/build/internal/relui: unskip TestRelease/rc test1w
Thanks.dmitshur reviewed +2 on doc: format net package release note1w
Thanks.dmitshur reviewed +2 on doc: inserted TODOs into 9-todo.md1w
Thanks.dmitshur reviewed +2 on doc: inserted TODOs into 9-todo.md1w
Thanks. I think this is good to merge as soon as the trybot failure is fixed. The improved format will make it easier to many people to follow up in parallel on addressing the TODOs, so no need for t…dmitshur commented on golang.org/x/exp/typeparams: update termlist, typeterm source files1w
> One caveat, seems like go mod tidy doesn't delete the empty go.sum file...
Ah, yeah. Maybe that's solvable with a 4th line like:
```
//go:generate rm go.sum # workaround for go.dev/issue/36236
`…dmitshur commented on sort: add go:fix inline directives to convenience wrappers1w
(1 comment)
This note was intentionally added as regular text for readers, rather than deprecating the symbol, to avoid creating churn.
Now that go:fix inline can reduce the toil in migrating from …dmitshur commented on golang.org/x/build/cmd/relui: add govulncheck-action repo tagging workflow1w
Thanks for working on this, and sorry about the review latency. In general, this looks good and makes sense. Thanks for implementing all the steps involved in tagging and following up.
I left a numb…dmitshur commented on golang.org/x/build/cmd/relui: define a govulncheck-action tagging workflow1w
It's good that you're taking into account that there is a general-purpose tagging workflow in relui (I was going to mention it in CL review too). That said, that workflow is intended to work on all o…dmitshur reviewed +2 on golang.org/x/build/cmd/gomoteserver: add Kubernetes health checks1w
Thanks.
This makes sense to me. My one minor comment is about possibly simplifying the probe configuration a bit, but I have no objections to any of it if it helps.
From briefly re-reading about ho…dmitshur closed a change golang.org/x/build: Expose more ReviewInfo fields in the Go Gerrit client1w
This PR is being closed because [go.dev/cl/234660](https://go.dev/cl/234660) was closed.
The x/build/gerrit [package comment](https://pkg.go.dev/golang.org/x/build/gerrit#pkg-overview) is now more…dmitshur commented on golang.org/x/build: Expose more ReviewInfo fields in the Go Gerrit client1w
This PR is being closed because [go.dev/cl/234660](https://go.dev/cl/234660) was closed.
The x/build/gerrit [package comment](https://pkg.go.dev/golang.org/x/build/gerrit#pkg-overview) is now more…dmitshur closed a change golang.org/x/build: Expose more ReviewInfo fields in the Go Gerrit client1w
dmitshur commented on os: notify testlog from (*File).Chdir1w
This should be covered at https://go.dev/wiki/MinorReleases. In particular, see this paragraph:
> When the child issue is labeled CherryPickApproved, the original author of the change fixing that is…dmitshur opened a change golang.org/x/website/go.mod: pull in "relnote: include file name for stdlib minor changes"1w
Pull in x/build CL 783780 so that its effect on the /doc/go1.27 page
applies sooner.dmitshur opened a change golang.org/x/build/internal/relui: unskip TestRelease/rc test1w
It's preferable to be able to run the TestRelease/rc test ahead of the
upcoming RC 1 release, and see that it passes. Make that possible with
a fairly small scope reduction in the FakeCloudBuild impl…dmitshur opened a change golang.org/x/build/internal/relui: only run TestRelease/major on 1.26 and newer1w
A fixup for CL 768223 to unbreak x/build tests with Go 1.25.dmitshur reviewed +2 on golang.org/x/exp/typeparams: deprecate and inline aliased functions1w
> as far as I know that is not considered an "API change" and so no proposal needed
I prepared CL 783580 but became less sure about that. After all, a similar argument may apply - adding a go:fix in…dmitshur commented on golang.org/x/exp/typeparams: update termlist, typeterm source files1w
I suspect it may work to make GO111MODULE=on work by taking advantage of go:generate directives being defined to execute in a predictable order, and doing something like:
```
//go:generate go get go…dmitshur commented on testing: allow examples to return an error1w
(Someone may extend that argument to say that it's odd to accept an error return from example and test functions, but not from net/http [handlers](https://pkg.go.dev/net/http#Handler).)dmitshur commented on cmd/go: check that insecure scheme is permitted by GOINSECURE1w
This change is ready for review.dmitshur commented on cmd/go: start a test case for an insecure scheme check1w
(1 comment)
This change is ready for review.
(nit) This should maintain a trailing newline. A self-reminder to remember to push a patch set with a fix.