Activity

Yesterday
dmitshur starred in github.com/google/iconvg13h
This Week
dmitshur starred in github.com/progrium/shelldriver1d
dmitshur starred in github.com/progrium/vizgo1d
(1 comment) Please add a Fixes line here pointing to an existing issue if it exists, otherwise a new issue.
Indeed. (I missed the earlier notification.) Closing this one.
(I've sent CL 367014 for that.)
This change is ready for review.
The comment is already inside a /* */ block, so no need for //. Updates golang/go#49212.
Thanks for sending this fix, however this command has been deprecated (see CL 349051), so I don't think we should try to continue to maintain it, other than to fix the deprecation notice (it has …
@zx2c4 We've discussed this on the release team, and in general we are okay with this happening for Go 1.18, especially if it can help fix other bugs. It seems the CLs are reviewed and ready to go, s…
dmitshur reviewed +1 on .github: use multiple issue templates2d
Thanks for addressing the review comments this has received so far Sean. With Thanksgiving Day in the US coming up tomorrow and many people being away, I think we should avoid submitting this for now…
It should be okay, you can submit this independently since it uses cherry-pick strategy and there aren't conflicts between the two CLs.
(1 comment) Add 'golang/go' before #, since this is an x/ repo.
dmitshur merged a change doc/go1.18: add Clone doc2d
Thanks for reporting. Updated. Unfortunately that means no more "Verified" badge for now, but we'll get that back after some time. Though go.dev is the new shorter URL where the Go websites have bee…
@changkun Yes, I think we'll go with iOS 12 for Go 1.18. I've mailed [CL 366914](https://golang.org/cl/366914) to document it in the release notes. Thank you for your help with with updating the buil…
For #47694. Updates #49616. Updates #48076.
FreeBSD and macOS updates look good. Based on today's discussion, I expect Windows builder for 1.18 may be changing, is that right? (I'm not working on the implementation beyond what's a…
dmitshur reviewed +1 on .github: use multiple issue templates3d
dmitshur reviewed +1 on .github: use multiple issue templates3d
Thanks. I primarily reviewed the 00-bug.md template, other than a few minor comments, it looks good. I looked over the rest, and didn't spot problems. Adding a few more people to give them a cha…
dmitshur reviewed +2 on vendor: update golang.org/x/net to tip3d
Thanks. Yes, we ended up with the go.mod files controlling both vendor and bundle versions, rather than managing them separately (issues #41375 and #41409 have more context).
dmitshur reviewed -2 on vendor: update golang.org/x/net to tip3d
When modifying versions selected in go.mod files, it is useful to run tests in the cmd/internal/moddeps package (or request a longtest builder that will also do that), as they help accidental version…
Thanks for sending this CL. I left some initial comments/questions. I've also asked pkgsite and gopls owners to take a look at those sections. These 2 blank lines were intentional: they allow …
This looks relevant to: ```Go // If a build fails multiple times due to communication // problems with the buildlet, assume something's wrong with // the buildlet or machine and fail the build,…
Thanks for the report. I can reproduce this too. CC @jamalc, @rsc.
Thanks, but to fix this issue, the playground.js file in x/website will need to be modified, not this old copy in x/tools. Please see my comment at https://go.dev/issue/49527#issuecomment-976799250.
The playground has recently moved to go.dev/play, and so the fix needs to happen in x/website. > it seems like the same fix is needed in `playground.js` in `tools/godoc/static` Oh, that was actuall…
(1 comment) That should've been: ``` ... to [golang.org/x/website/_content/talks/](https://golang.org/x/website/_content/talks). ``` (It seems there's a bug in Gerrit comment rendering, wh…
See inline comments. I suggest leaving only README.md file as we've done in x/blog. Otherwise LGTM. Please also include the information that it was CL 365135 and commit e219555f where it was add…
The x/talks repository followed suit after x/blog and x/tour and has been merged into x/website. CL 366177 removes all of its moved content, leaving just a README. Stop displaying it on the build da…
This change is ready for review.
(1 comment) This doesn't appear to be an intentional change. Sent CL 366554 to revert it.
This change was made inadvertently as part of CL 365495.
(1 comment) Shall this switch be removed?
Thanks, leaving to Russ or Jamal to review and submit.
Changes in PS 3 also look reasonable to me. Thanks for answering!
dmitshur edited the wiki4d
@uppalabharath Thanks for offering to help (be sure to look over https://go.dev/doc/contribute if you haven't already)! I think the message just needs to communicate the reason for the problem. Perh…
Also, the link is to "golang.org/dl/" but should likely be updated to "go.dev/dl/". CC @rsc.
(2 comments) Me too. In situations where fields from the error type are used, I typically use: if e := (sourcecache.TooBigError{}); errors.As(err, &e) { // use e.Foo } But I o…
(1 comment) I tried making it an unexported method on buildStatus. It was much better than inside a distant buildgo package, but it still felt worse than just inlining the 2 calls directly into runS…
Oh, I missed that it wasn't https://www.jsonfeed.org/, thanks for catching. Nit: no longer 'json'.
(1 comment) CL 366135 increases the x/website limit too 200 MB, and will resolve this.
Update on 2021-11-22: x/talks content (74 MB compressed) is being moved into x/website in CL 365135, pushing it over 100 MB. [CL 366135](https://golang.org/cl/366135) updates the limit for x/websi…
The x/website repository is unusually large due to having content from x/blog, and as of CL 365135 also from x/talks. Increase its limit to 200 MB, but leave other repositories as they were. Updates…
Fixing this involves starting to track the number of retries (for a high-level trybot request on a given repo/commit) due to communication errors, and modifying [`repeatedCommunicationError`](https:/…
The build system enforces a limit on the compressed tree tarball size that it's willing to fetch and propagate throughout the rest of the system. If that limit is exceeded, consider it a terminal…
(Also filed golang.org/issue/49707 to make coordinator stop after one such failure rather.)
If a CL pushes the (compressed) size of the tree over a limit, trybots fail with something like: ``` Error: runTests: reading website/b4aa6ae539a1f644d48c473ca6e0e31a95ab4b26 from gerrit: rejecte…
Removing trybot vote for now since they can't complete without some change; see inline comment. TryBots are currently unable to run because this CL pushes the size of a compressed tarball of the…
(1 comment) Ah. Thanks!
Last Week
(1 comment) Yep, it's right. The parent commit in this CL is 7c9fc1ca, which is a commit from CL 364276. 👍
(1 comment) Any reason not to include the Content-Type header check?
(1 comment) CL 364276 fixes this parameter to have the correct value, "go.dev". It'll require the merge conflict to be resolved here, since this RegisterFeeds call is moved (and its firs…
Thanks. If you wanted and if this is rebased on top of CL 364276, you can add something like this under the 'GET https://go.dev/blog/feed.atom' block: body contains <link rel="se…
Thanks for the fix, it looks right to me. Let's be sure to add a test case so we can be confident the feed(s) won't break in the future, see inline comment. Russ, please let us know if you h…
Thanks everyone for reporting this, it is indeed a new problem. It should be fixed within a couple of days, along with a test to avoid future regressions. Thanks for your patience.
@runeimp Thanks for context. It the header was indeed removed in some versions of Xcode and then brought back, that could explain how some people (and not everyone) ran into this. I'll close this …
(1 comment) CL 365095 modifies this adjacent line, so this may unfortunately generate a trivial merge conflict.
@tklauser Do you think it's safe to expect that 2021e will still be latest by the time 1.18 final is released (est. Feb 2022)? If there's a chance of a newer timezone release before then, we should m…
That's odd. The aforementioned program builds successfully (and prints "Hello Carbon." if executed) for me both on: - macOS Monterey (12.0.1), M1 CPU - macOS Big Sur (11.6.1), Intel CPU I expect th…
(1 comment) A question about this change. There's already a '/*' path that also maps to coordinator-internal backend on port 444. Are the new rules being added in this CL because that ru…
(1 comment) Both https://go.dev/blog/feed.atom and https://go.dev/blog/.json serve 404 right now. Sean, maybe there's been a change or regression since this CL was sent, perhaps in CL 362955?
If you'd like to proceed with this CL, it'll need to be rebased to resolve a conflict with CL 365318.
(1 comment) This should be /pkg/strings/#Trim since this entry is about the strings package. There is also an existing open CL 363840 that set out to document these changes, so if that CL is rebase…
A few minor comments on the commit message style, since you may be sending more CLs here, I hope they'll be helpful (new contributors often look only at the most recent CL and repeat its style). …
dmitshur reviewed +2 on doc/go1.18: add Clone doc1w
Thanks. Adding Jeremy who can review for trust and submit. This is resolved. Resolved. Resolved.
`gerritbot` currently checks whether each PR has a "cla: yes" label (using the maintner corpus, so it doesn't contribute to GitHub API rate limit use) to determine that CLA checks have passed before …
Minor comments, otherwise leaving to Alex to review. Maybe a good opportunity to update the slowbot aliases too. In this package, it's common to use the atLeastGo1(goBranch, 16) helper for perf…
(4 comments) Can include CL number to make it easier to navigate to it: ... in CL 354757 ... If I understand correctly, the issue happens much more often on GOARCH=386 than GOARCH=amd64 for Ope…
Take a small step to bring the dashboard towards the current state of the builder. For golang/go#49325.
(1 comment) This can be simplified to: buildsRepo: defaultPlusExpBuild, Without changing behavior, since by now 1.16+ are the only supported/tested branches, 1.15 and older are not.
Just leaving a minor first pass comment for now; I plan to read the rest of the change carefully in a second pass. This is the first time we're adding a "==" query and I'm not sure w…
Changes look good. +2 is for after inline comments are resolved. Is the new 13.0 image known to work as expected via some form of testing, or is the plan to deploy coordinator with CL 365314 to lear…
Thanks. Please use the existing format to mention a second CL here: <!-- CL 323318, CL 332771 --> This is so that existing tooling recognizes that these CLs have already been mention…
dmitshur commented on doc/go1.18: add Clone doc1w
(1 comment) Also, the link should be "/pkg/strings/#Clone" (it's currently missing an 's').
dmitshur reviewed +2 on doc/go1.18: add Clone doc1w
Thank you for sending this change. It's possible we can find ways to make the description easier for readers to understand in future editing passes (or maybe another reviewer will suggest so), b…
I think this is a good idea, though it might be preferable to do in a follow up change rather than all at once. However, it's also fine to just do it here. Leaving to Carlos to decide.
Leaving this to other reviewers more familiar with the context, but I'm not spotting any issues. It passes with today's gotip on darwin/amd64 for me (it didn't pass with a gotip from a we…
(CCing Michael just as FYI.)