[pkgsite] internal/frontend: migrate go-mod-viewer.appspot.com to pkgsite.go.dev/go-mod-viewer
Neal Patel (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
Jonathan Amsterdam voted and added 2 comments![]( "Open in Gerrit")
Votes added by Jonathan Amsterdam
| Code-Review | +2 |
2 comments
internal/frontend: migrate go-mod-viewer.appspot.com to pkgsite.go.dev/go-mod-viewerThere is pkg.go.dev, the website, and x/pkgsite, the repo.
After this CL is deployed, can you visit pkg.go.dev/go-mod-viewer/path/to/module? It looks that way.
Put that info in this message.
for (const el of document.querySelectorAll<HTMLSelectElement>('.js-highlight')) {What does this do?
Related details
- Neal Patel
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Neal Patel
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel added 2 comments![]( "Open in Gerrit")
internal/frontend: migrate go-mod-viewer.appspot.com to pkgsite.go.dev/go-mod-viewerThere is pkg.go.dev, the website, and x/pkgsite, the repo.
After this CL is deployed, can you visit pkg.go.dev/go-mod-viewer/path/to/module? It looks that way.
Put that info in this message.
Correct; I misspoke in the CL description.
Fixed.
for (const el of document.querySelectorAll<HTMLSelectElement>('.js-highlight')) {What does this do?
Preserves the client-sided highlighting based on the url fragment: https://github.com/rsc/swtch/blob/master/app/go-mod-viewer/viewer.js
Related details
- Jonathan Amsterdam
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
Jonathan Amsterdam voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jonathan Amsterdam
| Code-Review | +2 |
1 comment
for (const el of document.querySelectorAll<HTMLSelectElement>('.js-highlight')) {Neal PatelWhat does this do?
Preserves the client-sided highlighting based on the url fragment: https://github.com/rsc/swtch/blob/master/app/go-mod-viewer/viewer.js
- Neal Patel
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel voted Commit-Queue+1![]( "Open in Gerrit")
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
kokoro (Gerrit)
kokoro voted kokoro-CI-1![]( "Open in Gerrit")
Kokoro presubmit build finished with status: FAILURE
Logs at: https://source.cloud.google.com/results/invocations/2d23d09d-8147-4bea-aba5-c4f90514ab8b
| kokoro-CI | -1 |
Related details
- Neal Patel
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel voted Commit-Queue+1![]( "Open in Gerrit")
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil voted Code-Review+1![]( "Open in Gerrit")
- Neal Patel
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Alan Donovan added 1 comment![]( "Open in Gerrit")
internal/frontend: migrate go-mod-viewer.appspot.com to pkg.go.dev/go-mod-viewerCan we make this endpoint URL shorter? This was the name of an AppEngine app; we needn't preserve it here. For starters, the "go" part can clearly go. I don't know what endpoints already exist, but I wonder whether this can be something as short as /src or /mod or /view.
Related details
- Neal Patel
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Liao (Gerrit)
Sean Liao added 1 comment![]( "Open in Gerrit")
Maybe link to https://go.dev/issue/66432 and https://go.dev/issue/66653 which could be fixed in the future using this
Neal Patel (Gerrit)
Neal Patel uploaded new patchset![]( "Open in Gerrit")
- Code-Review: +1 by Damien Neil, +2 by Jonathan Amsterdam
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
- Damien Neil
- Jonathan Amsterdam
- Neal Patel
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Neal Patel
| Commit-Queue | +1 |
1 comment
internal/frontend: migrate go-mod-viewer.appspot.com to pkg.go.dev/go-mod-viewerCan we make this endpoint URL shorter? This was the name of an AppEngine app; we needn't preserve it here. For starters, the "go" part can clearly go. I don't know what endpoints already exist, but I wonder whether this can be something as short as /src or /mod or /view.
I chose `/view/`. Let me know what you think about the resulting file structure.
Related details
- Alan Donovan
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel added 1 comment![]( "Open in Gerrit")
Maybe link to https://go.dev/issue/66432 and https://go.dev/issue/66653 which could be fixed in the future using this
I took a cursory look at the issues; could you elaborate on how it might apply to https://go.dev/issue/66432?
I think I see how it applies to https://go.dev/issue/66653.
Related details
- Alan Donovan
- Neal Patel
- Sean Liao
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
kokoro (Gerrit)
kokoro voted kokoro-CI-1![]( "Open in Gerrit")
Kokoro presubmit build finished with status: FAILURE
Alan Donovan (Gerrit)
Alan Donovan added 7 comments![]( "Open in Gerrit")
After this CL is deployed, a module can be viewed in the following manner: https://pkg.go.dev/go-mod-viewer/rsc.io/qu...@v1.5.2view
(and throughout)
After this CL is deployed, a module can be viewed in the following manner: https://pkg.go.dev/go-mod-viewer/rsc.io/qu...@v1.5.2Presumably this displays a directory listing of the module (recursively?). What are the individual file URLs?
// GoModViewPage holds the information for a pagedelete throughout
j := strings.Index(urlPath[i:], "/")
if j < 0 {
mod, file = urlPath, ""
} else {
mod, file = urlPath[:i+j], urlPath[i+j+1:]
}strings.Cut
if resp.StatusCode != 206 {http.StatusPartialContent
But this could use some explanation.
resp.Body.Close()Why not defer this at L214 instead of repeating it 3 times?
n := 1 + bytes.Count(data, nl)
lines := make([]line, n)
wid := len(fmt.Sprintf("%d", n))
wid = (wid+2+7)&^7 - 2
n = 1// Round up width of maximum line number to next tab stop (8).
Related details
- Neal Patel
- Sean Liao
Neal Patel (Gerrit)
Neal Patel voted and added 7 comments![]( "Open in Gerrit")
Votes added by Neal Patel
| Commit-Queue | +1 |
7 comments
After this CL is deployed, a module can be viewed in the following manner: https://pkg.go.dev/go-mod-viewer/rsc.io/qu...@v1.5.2Neal PatelPresumably this displays a directory listing of the module (recursively?). What are the individual file URLs?
It provides the same view as https://go-mod-viewer.appspot.com/gonum.org/v1/go...@v0.16.0/stat
It doesn't recursively list (like `find`); rather, it's like `ls`.
After this CL is deployed, a module can be viewed in the following manner: https://pkg.go.dev/go-mod-viewer/rsc.io/qu...@v1.5.2Neal Patelview
(and throughout)
Done
// GoModViewPage holds the information for a pagedelete throughout
If making consistent throughout, then filenames of the static content should be changed as well as all the CSS class names and the references inside the template.
WDYT?
j := strings.Index(urlPath[i:], "/")
if j < 0 {
mod, file = urlPath, ""
} else {
mod, file = urlPath[:i+j], urlPath[i+j+1:]
}strings.Cut
I transposed all of rsc@ code without editing; do you think it's reasonable to modernize / optimize it in a follow up CL?
ctx: https://github.com/rsc/swtch/tree/master/app/go-mod-viewer/main.go
if resp.StatusCode != 206 {http.StatusPartialContent
But this could use some explanation.
This was similarly transposed from rsc@ source: https://github.com/rsc/swtch/tree/master/app/go-mod-viewer
See comment above about second pass in a follow up CL.
resp.Body.Close()Why not defer this at L214 instead of repeating it 3 times?
This was similarly transposed from rsc@ source: https://github.com/rsc/swtch/tree/master/app/go-mod-viewer
See comment above about second pass in a follow up CL.
n := 1 + bytes.Count(data, nl)
lines := make([]line, n)
wid := len(fmt.Sprintf("%d", n))
wid = (wid+2+7)&^7 - 2
n = 1// Round up width of maximum line number to next tab stop (8).
- Alan Donovan
- Sean Liao
Neal Patel (Gerrit)
Neal Patel uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
- kokoro-CI-Passes: kokoro-CI-1 by kokoro
Related details
- Alan Donovan
- Sean Liao
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Alan Donovan added 5 comments![]( "Open in Gerrit")
After this CL is deployed, a module can be viewed in the following manner: https://pkg.go.dev/go-mod-viewer/rsc.io/qu...@v1.5.2Neal PatelPresumably this displays a directory listing of the module (recursively?). What are the individual file URLs?
It provides the same view as https://go-mod-viewer.appspot.com/gonum.org/v1/go...@v0.16.0/stat
It doesn't recursively list (like `find`); rather, it's like `ls`.
Even better. Thanks.
// GoModViewPage holds the information for a pageNeal Pateldelete throughout
If making consistent throughout, then filenames of the static content should be changed as well as all the CSS class names and the references inside the template.
WDYT?
Yes, let's make it consistent.
j := strings.Index(urlPath[i:], "/")
if j < 0 {
mod, file = urlPath, ""
} else {
mod, file = urlPath[:i+j], urlPath[i+j+1:]
}Neal Patelstrings.Cut
I transposed all of rsc@ code without editing; do you think it's reasonable to modernize / optimize it in a follow up CL?
ctx: https://github.com/rsc/swtch/tree/master/app/go-mod-viewer/main.go
Ah, I didn't notice this code was just moving. Fine to leave as is.
if resp.StatusCode != 206 {Neal Patelhttp.StatusPartialContent
But this could use some explanation.
This was similarly transposed from rsc@ source: https://github.com/rsc/swtch/tree/master/app/go-mod-viewer
See comment above about second pass in a follow up CL.
Acknowledged
resp.Body.Close()Neal PatelWhy not defer this at L214 instead of repeating it 3 times?
This was similarly transposed from rsc@ source: https://github.com/rsc/swtch/tree/master/app/go-mod-viewer
See comment above about second pass in a follow up CL.
- Neal Patel
- Sean Liao
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel uploaded new patchset![]( "Open in Gerrit")
Related details
- Neal Patel
- Sean Liao
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel added 2 comments![]( "Open in Gerrit")
internal/frontend: migrate go-mod-viewer.appspot.com to pkg.go.dev/go-mod-viewerNeal PatelCan we make this endpoint URL shorter? This was the name of an AppEngine app; we needn't preserve it here. For starters, the "go" part can clearly go. I don't know what endpoints already exist, but I wonder whether this can be something as short as /src or /mod or /view.
I chose `/view/`. Let me know what you think about the resulting file structure.
Acknowledged
// GoModViewPage holds the information for a pageNeal Pateldelete throughout
Alan DonovanIf making consistent throughout, then filenames of the static content should be changed as well as all the CSS class names and the references inside the template.
WDYT?
Yes, let's make it consistent.
- Alan Donovan
- Sean Liao
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Neal Patel (Gerrit)
Neal Patel voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Alan Donovan
- Sean Liao
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
kokoro (Gerrit)
kokoro voted kokoro-CI-1![]( "Open in Gerrit")
Kokoro presubmit build finished with status: FAILURE
- Alan Donovan
- Neal Patel
- Sean Liao
Sean Liao (Gerrit)
Sean Liao added 1 comment![]( "Open in Gerrit")
Neal PatelMaybe link to https://go.dev/issue/66432 and https://go.dev/issue/66653 which could be fixed in the future using this
I took a cursory look at the issues; could you elaborate on how it might apply to https://go.dev/issue/66432?
I think I see how it applies to https://go.dev/issue/66653.
66432: the module source doesn't host a view of individual files. with this /view/, we'd be able to see the source code.
66653: the module source can lie about the file contents (e.g. tag was changed after proxy caching). with /view/, we'd always see what's in the proxy cache, matching the most common source for go get.
Related details
- Alan Donovan
- Neal Patel
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Liao (Gerrit)
Sean Liao added 1 comment![]( "Open in Gerrit")
internal/frontend: migrate go-mod-viewer.appspot.com to pkg.go.dev/go-mod-viewerNeal PatelCan we make this endpoint URL shorter? This was the name of an AppEngine app; we needn't preserve it here. For starters, the "go" part can clearly go. I don't know what endpoints already exist, but I wonder whether this can be something as short as /src or /mod or /view.
Neal PatelI chose `/view/`. Let me know what you think about the resulting file structure.
Acknowledged
Now that I think about it, pkgsite already has a source file view under /files/
https://cs.opensource.google/go/x/pkgsite/+/master:internal/frontend/server.go;l=215
maybe it should be unified with this?
Related details
- Alan Donovan
- Neal Patel
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
Jonathan Amsterdam voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
- Alan Donovan
- Neal Patel
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
kokoro-CI-Passes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |