Today
shurcooL commented on vfsgendev updates modification times in github.com/gopherjs/gopherjs5h
> even if it is using modification times to determine staleness, then when reading from the VFS it will always get the same answer Yes, it will be the same—but non-zero—answer. It would change…
shurcooL commented on compiler: remove dependency on lega… in github.com/gopherjs/gopherjs5h
> Incidentally, if there are tests/checks that you perform locally that help to give you confidence that this PR "works" we should look to add those tests/checks (or equivalent tests/checks) to the C…
shurcooL commented on vfsgendev updates modification times in github.com/gopherjs/gopherjs5h
> But the only thing we *need* to rely on here is the contents of those files; the modification times are irrelevant aren't they? Is that the case? I'm not sure, but I suspect they might be used by …
Yesterday
shurcooL commented on Robust Header Check in github.com/bradleyfalzon/ghinstallation8h
> So essentially, if the request already has an Accept header, and it's equal to `application/octet-stream`, don't send the `application/vnd.github.machine-man-preview+json` header? That's right. …
shurcooL commented on compiler: remove dependency on lega… in github.com/gopherjs/gopherjs8h
This line uses a very outdated style of initializing a new `bytes.Buffer`, no one writes it like that in anymore. [`bytes.NewBuffer`](https://godoc.org/bytes#NewBuffer) documentation hints at this: …
shurcooL commented on vfsgendev updates modification times in github.com/gopherjs/gopherjs9h
Yes, this is a known limitation of `vfsgen`, and the tracking issue for it is https://github.com/shurcooL/vfsgen/issues/26. > Am I right in thinking this is because the modification times are taken …
shurcooL commented on Robust Header Check in github.com/bradleyfalzon/ghinstallation10h
At a high level, this makes sense. I think the implementation can hopefully be made simpler. @bradleyfalzon See a relevant issue https://github.com/google/go-github/issues/870 for more context.
shurcooL commented on intermittent test failures / race c… in github.com/kardianos/osext10h
> I just checked again, and this issue is still present. But the README says: > As of go1.8 the Executable function may be found in `os`. The Executable function in the std lib `os` package is…
shurcooL pushed to master in dmitri.shuralyov.com/website/gido1d
07f8731647088509ad1be06a6fe85ce0ea2347b6Add support for Gerrit CL hashtags.
shurcooL commented on compiler: fix handling of struct, a… in github.com/gopherjs/gopherjs1d
Thanks for working on this bug, and sorry it's taking so long. I've found some time and started reviewing this now. I will need more time, but I found something that I haven't seen mentioned so far. …
Last Week
shurcooL commented on Commaf which rounds to X decimal pl… in github.com/dustin/go-humanize1d
Was this issue resolved by PR #60 (which has been recently merged), or is there more to do here? /cc @bramp
shurcooL commented on Add a few funcs for limiting the nu… in github.com/dustin/go-humanize1d
For posterity (since it's not very visible), this was merged via commit 41f808901d8eb7d8d4e70a79b62b0d1b504e3f9a.
shurcooL commented on X-Content-Type-Options: nosniff wil… in github.com/gopherjs/gopherjs1d
Thanks for helping diagnose the issue @myitcv. It sounds like it's resolved now, the problem was that the HTML included a script with the wrong URL, which gave 404. I'll close this because it's no…
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1d
As a followup step, I've checked that the GopherJS Playground works with the latest version (it does) and updated it in https://github.com/gopherjs/gopherjs.github.io/commit/cbdec896dfc8a335278926fc7…
shurcooL pushed to master in github.com/gopherjs/gopherjs.github.io1d
cbdec896dfc8a335278926fc7d6a7e2c43ccdb05playground: Regenerate with GopherJS 1.10-3 and Go 1.10.1 (on darwin/amd64).
shurcooL commented on Depend on Go 1.10, stop using tmp/ … in github.com/perkeep/perkeep1d
@mpl That PR (https://github.com/gopherjs/gopherjs/pull/787) has been merged, so you can switch to using GopherJS 1.10-3 (with support for being vendored) from `master` branch now.
shurcooL deleted branch in github.com/gopherjs/gopherjs1d
embed-core-pkgs
shurcooL pushed to master in github.com/gopherjs/gopherjs1d
1daac00a5961469351b87e7ac223239c37938d2acompiler: Bump version to GopherJS 1.10-3.
c8e5f7ce3f59b94ba3a11134fa53f2e561bd2596README: Remove note saying vendoring GopherJS is unsupported.
d0d69c08b6af458cebb60d3295f643e4a471029dtests: Add test that GopherJS can be vendored.
b24e3563d0efa33b32250411d469ba59fa5cf254Restore support for testing js package.
b90dbcb0f8851c1daa21542996b5f23d9779f720build, compiler/typesutil: Don't use vendor directory to resolve js, nosync.
c121b3d6abdf0d7534656ac098f9ae8c3c204abebuild: Load js, nosync packages from gopherjspkg rather than GOPATH.
a4af087c07864b4fe10e2c086e3cbca697f91adacompiler/gopherjspkg: Add package.
f681903bc8cc21f8940c141d2beb8660511bed5abuild: Make fewer copies of NewBuildContext.
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1d
Let's discuss that outside this PR. I've already effectively allocated 1.10-3 for this fix by bumping to 1.10-3 in the earlier version of this PR, so that release has already been decided. We can …
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1d
There isn't a need, that PR is orthogonal. We can bump up to 1.10-4 for it if needed.
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1d
Thanks for the reviews! I'm going to merge this and bump up the GopherJS compiler version to 1.10-3, the first version to support vendoring GopherJS into other Go projects.
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1d
The motivation is written in the commit message of d0d69c08b6af458cebb60d3295f643e4a471029d: > Rely on runtime.GOARCH and t.Skip to skip tests that shouldn't be run > with GopherJS. It's more cle…
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1d
Thanks for the suggestion. I wasn't familiar with this syntax, but I'll consider it next time.
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs2d
I've rebased this PR on top of latest `master` and addressed all outstanding comments. PTAL. (Previous PR version was 2ad4a5b9a30e5f1f31c643c2a2ac27543a5ddb58).
shurcooL pushed to embed-core-pkgs in github.com/gopherjs/gopherjs2d
c8e5f7ce3f59b94ba3a11134fa53f2e561bd2596README: Remove note saying vendoring GopherJS is unsupported.
d0d69c08b6af458cebb60d3295f643e4a471029dtests: Add test that GopherJS can be vendored.
b24e3563d0efa33b32250411d469ba59fa5cf254Restore support for testing js package.
b90dbcb0f8851c1daa21542996b5f23d9779f720build, compiler/typesutil: Don't use vendor directory to resolve js, nosync.
c121b3d6abdf0d7534656ac098f9ae8c3c204abebuild: Load js, nosync packages from gopherjspkg rather than GOPATH.
a4af087c07864b4fe10e2c086e3cbca697f91adacompiler/gopherjspkg: Add package.
f681903bc8cc21f8940c141d2beb8660511bed5abuild: Make fewer copies of NewBuildContext.
fcfa75a46cca3b1ed8b2c75c4f6f0a88df581baaMove js package tests into tests package.
e1d10e7424f54589aa3caa8f87ce2934d1890e45compiler: automate regeneration of prelude (#784)
27662f8dae2790b8232164d7570f54ba76144bc3CI: Update to Go 1.10.1. (#795)
e14987c0ef06db387b90fec85e8d06dc05598e24compiler: support arbitrary value js struct tag values (#779)
shurcooL deleted branch in github.com/gopherjs/gopherjs2d
embed-core-pkgs-followups
shurcooL deleted branch in github.com/gopherjs/gopherjs2d
master-with-vendor-test
shurcooL pushed to embed-core-pkgs-followups in github.com/gopherjs/gopherjs2d
c8e5f7ce3f59b94ba3a11134fa53f2e561bd2596README: Remove note saying vendoring GopherJS is unsupported.
d0d69c08b6af458cebb60d3295f643e4a471029dtests: Add test that GopherJS can be vendored.
shurcooL pushed to embed-core-pkgs-followups in github.com/gopherjs/gopherjs2d
c76cbeaf7eace8fee89a7727ac6f35b192ab29ddREADME: Remove note saying vendoring GopherJS is unsupported.
5e934b8d8dc160977db4e95e42458cf7e93315a6tests: Add test that GopherJS can be vendored.
shurcooL pushed to embed-core-pkgs-followups in github.com/gopherjs/gopherjs2d
7e4ffb231f626cded58652e7a9a9fd6d8ca2549cREADME: Remove note saying vendoring GopherJS is unsupported.
a43cd7adc28162c1353786f2ce5b68984e68dbactests: Add test that GopherJS can be vendored.
b24e3563d0efa33b32250411d469ba59fa5cf254Restore support for testing js package.
b90dbcb0f8851c1daa21542996b5f23d9779f720build, compiler/typesutil: Don't use vendor directory to resolve js, nosync.
c121b3d6abdf0d7534656ac098f9ae8c3c204abebuild: Load js, nosync packages from gopherjspkg rather than GOPATH.
a4af087c07864b4fe10e2c086e3cbca697f91adacompiler/gopherjspkg: Add package.
f681903bc8cc21f8940c141d2beb8660511bed5abuild: Make fewer copies of NewBuildContext.
fcfa75a46cca3b1ed8b2c75c4f6f0a88df581baaMove js package tests into tests package.
e1d10e7424f54589aa3caa8f87ce2934d1890e45compiler: automate regeneration of prelude (#784)
27662f8dae2790b8232164d7570f54ba76144bc3CI: Update to Go 1.10.1. (#795)
e14987c0ef06db387b90fec85e8d06dc05598e24compiler: support arbitrary value js struct tag values (#779)
shurcooL deleted branch in github.com/gopherjs/gopherjs2d
move_js_tests
shurcooL pushed to master in github.com/gopherjs/gopherjs2d
fcfa75a46cca3b1ed8b2c75c4f6f0a88df581baaMove js package tests into tests package.
shurcooL commented on Move js package tests into tests pa… in github.com/gopherjs/gopherjs2d
I'm going to merge this and rebase #787 on top. That way, none of the GopherJS tests will be embedded into the core packages, which has benefits (smaller compiler size, easier to modify tests).
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs2d
I often prefer Go over other tools, but a bash script seemed like a good fit for this task. It was based on the bash script in [playground repo](https://github.com/gopherjs/gopherjs.github.io/blob/b3…
shurcooL commented on Move tests in js package to tests p… in github.com/gopherjs/gopherjs2d
We already have that, except it's a build tag. It's `gopherjsdev`. If you build GopherJS with `go build -tags=gopherjsdev`, it reads from disk rather than using embedded version. I've considered t…
shurcooL created branch in github.com/gopherjs/gopherjs2d
embed-core-pkgs-followups
shurcooL commented on Move tests in js package to tests p… in github.com/gopherjs/gopherjs2d
> The issue with #800 is that the js tests are now further from the js package. Many tests in `tests` package also test `js` and other misc GopherJS behavior. Now they're all in one place. Further…
shurcooL commented on Move tests in js package to tests p… in github.com/gopherjs/gopherjs2d
Not seeing any surprises with them move so far. Sent #800.
shurcooL created branch in github.com/gopherjs/gopherjs2d
move_js_tests
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs2d
I can make it something like this, hope it's more clear: ``` unexpected stdout from gopherjsvendored_test.sh: got: unexpected output 123 want: hello using js pkg ```
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs2d
That's out of scope of this PR. This `gopherjspkg` package is the same in structure as `natives`.
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs2d
I don't see a difference, but sure, I can take out this commit and apply it on `master` after this PR lands.
shurcooL commented on Move tests in js package to tests p… in github.com/gopherjs/gopherjs2d
> but why are we embedding the tests in the first place? Because a package consists of normal .go files and _test.go files. I embedded the entire package, because any other approach would be more …
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs2d
Did you see the commit message of 49f2b5ca9ae7415de29eb86c95d2cecaca9d606f? However, I think I know what it'll take to make it work with `sh`. It might just be a matter of removing the `SIG` prefi…
shurcooL commented on client code can request the VM to r… in github.com/go-interpreter/wagon2d
What if it's neither of those two types? Then `err` would be left `nil`.
shurcooL commented on multiple static paths in github.com/shurcooL/vfsgen2d
Have you seen package [`github.com/shurcooL/httpfs/union`](https://godoc.org/github.com/shurcooL/httpfs/union)? For example, see it being used in [here](https://github.com/shurcooL/issuesapp/blob/…
shurcooL pushed to master in github.com/google/go-github2d
9b5056ba738bf2474d064620d54b708941b6162eAdd example for OrganizationsService.ListTeams. (#896)
shurcooL commented on Sequencing PRs to be merged in github.com/gopherjs/gopherjs3d
Quick thoughts on prelude changes. I don't quite see why we need or want #794. Shouldn't the formatting of .js files be done by a local tool? I don't think we win much by being so strict about for…
shurcooL commented on Add example to get a team ID from i… in github.com/google/go-github3d
There are many valid ways of doing pagination and detecting last page. This is one of them. But I think we should be consistent with the example provided at https://godoc.org/github.com/google/go-…
shurcooL commented on Support preview Team Discussions API in github.com/google/go-github3d
Similarly, what do you think of just making `comment` a value rather than pointer here as well?
shurcooL commented on Support preview Team Discussions API in github.com/google/go-github3d
We did the `if pull == nil` change for existing endpoints where a pointer was already used (see PR #539), in order to avoid making a breaking API change. Since this is a new API, I think we can si…
shurcooL commented on detect abuse rate limits on github … in github.com/google/go-github3d
googlebot's concern is not an issue; I obviously consent. Merged as dcc59e6. Thanks @bbatha! When you hear any updates on the ticket you submitted, please post an update here.
shurcooL pushed to master in github.com/google/go-github3d
dcc59e61eb134dd4eb92bdeaacedfdcd88d58ad0Detect abuse rate limits on GitHub Enterprise instances. (#865)
shurcooL pushed to patch-1 in github.com/bbatha/go-github3d
b028b5649c37cbb2682327f80bbf3122fde8f690Change "github enterprise" to "GitHub Enterprise".
shurcooL commented on detect abuse rate limits on github … in github.com/google/go-github3d
And on this line too.
shurcooL commented on detect abuse rate limits on github … in github.com/google/go-github3d
I believe Glenn was referring to the string here, which can be made "GitHub Enterprise".
shurcooL pushed to master in github.com/gopherjs/gopherjs3d
27662f8dae2790b8232164d7570f54ba76144bc3CI: Update to Go 1.10.1. (#795)
shurcooL commented on WIP: WebcryptoAPI support in github.com/gopherjs/gopherjs3d
About `//gopherjs:keep_overridden`, is it possible to defer that until a future PR? A lot of code GopherJS already deals with preventing DCE by assigning to `_` (search for "DCE" in the codebase).…
shurcooL commented on compiler: automate regeneration of … in github.com/gopherjs/gopherjs3d
This isn't a big deal, but it's pretty unusual to see a `[]byte` variable named `byts`. As far as I understand, it's a shortened version of "bytes"? It comes up a total of 5 times in the Go project. …
shurcooL commented on compiler: automate regeneration of … in github.com/gopherjs/gopherjs3d
`bpkg.Dir` is not a very readable name here. I would've copied the `importPathToDir` helper from: https://github.com/gopherjs/gopherjs/blob/e14987c0ef06db387b90fec85e8d06dc05598e24/compiler/native…
shurcooL commented on proposal: cmpopts: add convenience … in github.com/google/go-cmp3d
@dsnet What you've described sounds quite positive and has nice properties. I just want to point out, at least with the example given, I don't see why `cmp` itself is needed to achieve that functi…
shurcooL commented on goimports doc lists out-of-date go-… in github.com/golang/gddo3d
This issue tracker is for the gddo (godoc.org) project itself. You're referring to a package in the `x/tools` subrepo. People who work on `goimports` will not see it. The right issue tracker is for …
shurcooL commented on x/build/cmd/gopherbot: too close-ha… in github.com/golang/go3d
Yet another thing I noticed recently that I suspect is related. In issue #24850, if you look at the [issue events API](https://api.github.com/repos/golang/go/issues/24850/events), you can see goph…
shurcooL commented on x/build/maintner: GerritMessage doe… in github.com/golang/go4d
Fair enough, but the tradeoff is more data. Are you okay with the size increase from including raw `git diff-tree` output for meta commits? Should we try to estimate or calculate how much it wo…
shurcooL commented on x/build/maintner: GerritMessage doe… in github.com/golang/go4d
Posting some notes here after a quick investigation into where the inline comment data is. It's available via Gerrit's git protocol (unsurprisingly). It's located inside the diff of the correspond…
shurcooL starred github.com/mjl-/duit5d
shurcooL commented on 75a3424a05416492a2d6310c57fb965167651b35Recommit feedback for @… in github.com/gopherjs/gopherjs5d
@myitcv Thanks for elaborating on the tradeoffs involved. That makes sense to me. I will take that into consideration next time before making this suggestion. > You are both, I think, wanting to g…
shurcooL commented on bzip2: merge into Go standard libra… in github.com/dsnet/compress5d
@Darren I see your point. I expect/hope this issue will eventually be resolved, when the conditions for it are right.
shurcooL commented on detect abuse rate limits on github … in github.com/google/go-github5d
Thanks for the ping @bbatha, we just forgot about this PR. I've requested a review from @gmlewis and we can merge once it looks good to him as well. One more thing, have you had a chance to report…
shurcooL commented on Mutation struct variable empty? in github.com/shurcooL/graphql5d
Thanks for following up. I'm glad you got to the bottom of this. I think I understand what happened now. This general GraphQL client package was factored out of `graphql`, the GitHub-specific GraphQ…
shurcooL commented on 75a3424a05416492a2d6310c57fb965167651b35Recommit feedback for @… in github.com/gopherjs/gopherjs5d
I agree with @hajimehoshi, this should be `cmd.Output`. > In the case of normal operation `CombinedOutput` == `Output` This is not always true. Stderr can contain warnings or other information…
shurcooL commented on bzip2: merge into Go standard libra… in github.com/dsnet/compress6d
> Surely it's more important that there is a decent implementation available in the standard library now, rather than a potential future "best" implementation? If this package has the needed functio…
shurcooL commented on Return an error on Unreachable inst… in github.com/go-interpreter/wagon6d
I don't have any further feedback beyond what I said above. I defer to the project owner on this.
shurcooL commented on Implement focus callback in github.com/goxjs/glfw6d
Thanks for the new PR. I'll able to test and review it about a week from now, sorry about the delay (busy time for me). Feel free to ping in case I forget.
shurcooL commented on Depend on Go 1.10, stop using tmp/ … in github.com/perkeep/perkeep6d
@mpl There are some minor outstanding review comments I plan to address, but I won't have a chance to work on it until closer to end of the week or the weekend. So that's likely when it'll get merged…
shurcooL commented on Support new preview Label API impro… in github.com/google/go-github6d
This has been implemented via PR #866 and merged as commit 64f8f21e24feb72595a384afcf66f9caa7106fbf. Thanks @gjacquet!
shurcooL commented on compiler: support arbitrary value j… in github.com/gopherjs/gopherjs1w
Thanks again @myitcv!
shurcooL deleted branch in github.com/shurcooL/go-vcs1w
fix-ci
shurcooL pushed to master in github.com/gopherjs/gopherjs1w
e14987c0ef06db387b90fec85e8d06dc05598e24compiler: support arbitrary value js struct tag values (#779)
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1w
The failure output is: ``` $ go test -v -run=TestGopherJSCanBeVendored === RUN TestGopherJSCanBeVendored --- FAIL: TestGopherJSCanBeVendored (17.78s) gorepo_test.go:43: got != want: got:…
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1w
The test is considered failed if the exit code is non-zero, or if the stdout output is not exactly the expected "hello using js pkg\n" value. Stderr can be anything, it shouldn't fail the test. It…
shurcooL commented on Embed core GopherJS packages into b… in github.com/gopherjs/gopherjs1w
Yeah. Perhaps this script can be improved in the future to infer dependencies more automatically (e.g., with `go list -f '{{join .Deps "\n"}}' github.com/gopherjs/gopherjs/...` or so). There's a c…
shurcooL commented on 5ade898a39d8739bbe2c67f7776af633204d1ddaApply minor style and d… in github.com/gopherjs/gopherjs1w
It's used in `funcContext.newVariableWithLevel`, `funcContext.translateToplevelFunction`, and `Compile`. Looks like it's for top-level identifiers or so. I guess it's okay for it to change the nam…