Activity

Today
(1 comment) Would it be better to use const here?
Thanks for looking into this @ipriver. The underlying problem is a problem in the OS detection code. Let's close this as duplicate of #41232, and use #41537 to track this issue.
Yesterday
(1 comment) I'm going to revert the change to src/README.vendor for now (there are further changes to updatestd I want to consider after a discussion with Bryan, so it's too early to be advertising …
When issuing a minor Go release, we can consider patch updates that may be available to dependencies. For example, suppose that Go 1.18.3 standard library uses `golang.org/x/mod` at version v1.5.0…
(1 comment) This has changed a bit in PS 3 and PS 4, mind reviewing again please, including the new change to src/README.vendor? Thanks.
Moved the program to x/build, see CL 256357 instead. Will merge the change to src/README.vendor into CL 255860 instead.
We need to update dependencies vendored into the standard library at least two times during each major Go release cycle. We also need to do this whenever there are fixes backported to release branche…
(1 comment) Thanks!
(1 comment) Two small comments here: The common style in this file is to check if issue has a label, and then remove it. If you don't do the check, it will write messages like "Issue %d (in maintne…
(1 comment) Adding it is okay, but please move it into isDocumentationTitle.
(3 comments) Does -dry-run output look okay? (FWIW, I initially considered suggesting to re-inline this variable (like it was before CL 255939), but it's probably better to keep this as is so that …
(1 comment) Oh, I see this builds on top and modifies the label.
(1 comment) How does this relate to CL 255939?
Last Week
dmitshur starred in github.com/thommil/tge2d
If I understand this issue correctly (also based on reading https://github.com/darkfeline/animanager-go/issues/1), this issue doesn't exist in module mode, only in GOPATH mode. As a consequence, t…
(1 comment) The test failure is because findGorootModules in moddeps_test.go calls 'go list -json -m' to determine the main module path and dir, but that fails because 'go list -m' tries to compute …
(2 comments) PTAL. As a note, I'm primarily looking to submit CL 255860 sometime soon (on the order of days is fine), but not in a rush to get this in if you have more feedback or want to look for …
(2 comments) Done in PS 3. (Luckily both 1.15 and 1.14 release branches are in good shape, so no need to special-case skip it.) I'll go without vendoring in patch set 3 to show what it looks like.…
Thanks for reporting. This seems to also apply to Windows. It's possible to select a zip (e.g., go1.15.2.windows-amd64.zip) at https://golang.org/dl/, but the https://golang.org/doc/install#install s…
(4 comments) I considered that but didn't find a reason to include it here at this time. They're partially ignored in the context of packages, but not for module boundaries. E.g., github.com/TarsClo…
(1 comment) I should add that I intentionally used the Gerrit API here to avoid making caching done by the module mirror a factor, or being forced into turning off the module mirror when using the g…
(4 comments) Thanks for the detailed review Bryan. I'll consider and respond to the rest of the comments later, but I wanted to reply to a subset sooner. The version selection is done by making a …
dmitshur closed a change : [release-branch.go1.13] empty change3d
The test is over, this empty CL is not needed now.
(1 comment) s/repos/issues/
(2 comments) Should have golang/go# prefix since it's not the main repo. This expression can be simplified to the constant "true", which I don't think is intended. It can be fixed in place (e.g., …
The `TestScript/test_exit` test in `cmd/go` failed on the `linux-386-longtest` builder in https://build.golang.org/log/6f34555f06c618921fe4b9e85fbe1522a1a8078e because a panic wasn't seen: ``` --…
dmitshur merged a change golang.org/x/perf/vendor: delete4d
Submitting this now as I believe it's safe, the newly selected versions are functional. If this does cause some issue, please ping me and I'll look into it.
dmitshur commented on golang.org/x/perf/vendor: delete4d
(1 comment) TRY=longtest, linux-386-longtest
dmitshur commented on golang.org/x/perf/vendor: delete4d
(1 comment) TRY=longtest
(1 comment) Thank you for your work as an owner.
Thanks, but I think this issue was already reported earlier in #292.
(1 comment) Done.
(2 comments) This needs a copyright header. The next patch set will add it. This patch set vendors the single package from x/build here. It makes the CL more verbose. I've considered the alternati…
(1 comment) TRY=js, s390x, netbsd-amd64, aix, windows-amd64-longtest, darwin-amd64-10_15, linux-amd64-longtest, linux-386-longtest
The Go 1.16 development cycle has started. This is the time to update all golang.org/x/... module versions that contribute packages to the std and cmd modules in the standard library to latest master…
We need to update dependencies vendored into the standard library at least two times during each major Go release cycle. We also need to do this whenever there are fixes backported to release branche…
(1 comment) I followed the pattern shown in the example at https://pkg.go.dev/golang.org/x/tools/go/packages/packagestest#hdr-Example, and this is commonly done in x/tools (e.g., https://github.com/…
> Indirect golang.org/x dependencies are not updated, although they could be. I'm going with the more conservative choice for now. I'm open to feedback on this, and can see pros/cons to both strategi…
I'm creating a short program that performs this task. Using a program makes it easier to precisely define our update policy, and adjust it over time as needed. It also makes this task less error pron…
Thanks for the thoughtful feedback @ALTree and others. Even though the "go-approvers" group is documented as "People who can edit issues in the "go" repo", in reality some people in it take up muc…
Given that we recently went through a change on Gerrit side for #40699 (/cc @rsc @andybons), I wanted to revisit this now. This issue has received favorable feedback so far. > We should investigat…
(1 comment) (Minor.) A blank line needs to be added above the package clause to prevent the copyright comment from being considered a package comment.
> It's not a complete English sentence. An in-between solution that addresses this could be something like: "Go {{.V}} was released on {{.Date}}. See the [Release History](https://golang.org/doc/de…
dmitshur starred in github.com/u-root/gobusybox5d
(1 comment) Thanks for sending a CL. The issue this is fixing is currently in NeedsInvestigation state. I posted a comment there. Let's discuss this and agree we want this change first.
Copying a [comment](https://go-review.googlesource.com/c/website/+/254538/1#message-3d30dcbeb58ff148d105f0dc419a1cf3e9284e88) from the CL: > Here's a screenshot of what the addition to the page lo…
I don't understand this issue. When the -dst flag is not specified, bundle uses the package in the current working directory. The `go generate` command always sets the working directory to that of th…
(2 comments) Minor and optional suggestion to improve the CLI/clarity of documentation. Giving an optional flag a non-zero default value feels incompatible with the phrasing on top: > By default b…
As of CL 189818, bundle has been updated to use the go/packages API to load packages. That API supports module mode and legacy GOPATH mode. Update the test to provide coverage for all modes. Updates…
My understanding is that the bundled version and vendored version are currently specified independently. The bundled version wasn't updated for the Go 1.15 release. I think this issue still exists…
We've had issues in the past where version skew could be introduced if people forgot to update a one part of the distribution without also updating another (e.g., versions in src/go.mod and src/cmd/g…
(1 comment) Thanks for adding yourself!
(3 comments) I think this is a great change. There's more we can do later, but this should work well. Once this is pulled into x/website, this will also expose some .md files in the main Go tree, b…
@gurpreetz Thanks. That should not be happening, I'll need to investigate. Can you please file a new issue at https://github.com/shurcooL/githubv4/issues and specify what version of `golang.org/x/oau…
https://github.com/fydy/mirror_issue/blob/0b2042527c6dd994d2a512b2dd61623468975330/issues/source.go#L8 The canonical import of that package [has changed](https://github.com/shurcooL/githubv4/issue…
> I was already using those methods to authenticate when Github flagged it as soon to be deprecated - hence the request. Can you please elaborate? Can I reproduce this somehow? I've looked at h…
The best practice recommends avoiding global variables when it is possible and when it makes sense. It doesn't seem very applicable in this situation. To answer your question, let me be more preci…
dmitshur pushed to master in github.com/shurcooL/githubv41w
bc5e4feb2971a151ff59af94df558875d5c79aa4link to docs.github.com GitHub documentation site
LGTM. Thanks.
dmitshur pushed to master in github.com/shurcooL/githubv41w
98d25d370f1a1a42cf157717d391341d1e95a6b5regenerate for schema changes by 2020-09-14
Thanks. LGTM. > Reference: https://developer.github.com/v4/changelog/. It looks like https://docs.github.com/en/graphql/overview/changelog is a [better](https://github.blog/2020-07-01-launching…
FYI, there is a tracking issue golang/go#39883 for making that API available to for external use; mentioning it here so that this use case can be considered as part of that work. You may also want to…
dmitshur commented on all: add macOS ARM64 port1w
Thanks for making this proposal. This corresponds to the following requirements in https://golang.org/wiki/PortingPolicy#requirements-for-a-new-port: - A proposal must be filed and accepted in whi…
(1 comment) Thanks for the notice.
(3 comments) Can add: Fixes golang/go#34378. Something looks off here. [-nokeycheck] is repeated twice on this line.
dmitshur commented on all: add GOOS=ios1w
(3 comments) Is it in scope of this CL to update this documentation? (In case it is helpful, take a look at a past CL where this was done, https://go-review.googlesource.com/c/go/+/239278/1/doc/ins…
dmitshur reviewed +2 on misc/ios: quote paths1w
(2 comments) Should [-trust] be added here? Should [-trust] be added here?
Earlier
Can you please expand a bit on the motivation? What would you do differently if this were implemented? Is this a feature request to be able to defer the cost of initialization of the virtual filesys…
Thanks. The changes in the README.md file look good. Can you please also update one URL in doc.go?
(1 comment) I can reproduce an error when clicking the link above. It seems to happen only when visiting the URL with http:// scheme. There's no issue when using https://. A screenshot of the probl…
I'll close this CL since issue #21051 has been fixed via CL 173722 (as Caleb noted above), so this CL is no longer needed. If there's something left to do here, please feel free to re-open.
(2 comments) This is a change that has a chance to cause issues in some environments, either right away, or perhaps in the future. It's worth having an issue so people can find it and comment on it …
(1 comment) I don't know if you still want to proceed with this, but if so, this looks good. We can always remove this flag if it becomes no longer useful thanks to other future improvements.
(3 comments) Is it viable to also do this for Is, As, and Unwrap (from errors package in Go 1.13's standard library)? Is it out scope for this CL? A copyright comment is missing. A small nit on th…
I understand this issue is related to but not the same as #38751. The difference here is that it happens specifically for subprocess output even when the output is not large. Is that right?
dmitshur commented on all: add openbsd/mips64 port1w
@4a6f656c I’m not sure. According to https://github.com/golang/proposal#proposal-review, proposal review meetings happen approximately weekly, but this week may be unusual. @andybons Do you have…
@randall77 For a backport issue to get to CherryPickApproved state, someone on the release team needs to approve it. We've discussed this issue in past release meetings but a decision hasn't been mad…
This issue was open because [CL 252179](https://golang.org/cl/252179) has not been submitted yet. I'll reopen to track that (otherwise we risk shipping Go 1.16 without the fix). I left a ping commen…
(1 comment) Friendly ping on this CL. It has a +2 and one unresolved minor comment. It may need to be rebased because of a merge conflict.
(1 comment) The backport and removal of the compiler check look good to me, but I'd like someone more familiar with the compiler to review it.
I've answered more generally on the thread. The two backport CLs need to be reviewed, as they can't be submitted without a +2. Could you please find a code reviewer for them? Thank you.
Right now, the milestones of open backport issues that did not get included in a given minor release are automatically bumped to the next minor release milestone by `releasebot` at the end of a succe…
Approved as this is a test fix, needed to reduce false-positives during pre-release testing. This backport applies to both 1.15 (#41317) and 1.14 (this issue).
Approved as this is a test fix, needed to reduce false-positives during pre-release testing. This backport applies to both 1.15 (this issue) and 1.14 (#41322).