-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/pkgsite: javascript links to wrong lines (cs.opensource.google) #51624
Comments
I've submitted this feedback to cs.opensource.google. We can leave this open for visibility, but this is out of scope for pkgsite. |
Thanks, I wasn't sure where to raise it |
I tested this this morning and things are still a little confused That is I go to https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1792 Click on "QueryRowContext" and am now taken to Which has the correct definition for the method, but the version of the code is different ( a look at the page shows that it should instead be linked to https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1777 ) |
This is a limitation in https://cs.opensource.google/. The cross-reference metadata is generated at one commit. In this case, it is currently 47f806c. This updates about once per day. When browsing at a different commit [1], the UI still does its best to match references despite source skew. However, to try to link to the right location, it always links to code at the cross-reference commit (this the When browsing the Go 1.17 code, this limitation is certainly rather annoying since the code skew can be quite high. I think https://cs.opensource.google/ would like to be able to address this and support multiple cross-reference metadata commits (one per release branch, perhaps?), but I don't know if / when that will ever come. P.S. in your original issue comment, I'm not sure why you ended up with a cross-reference link missing the [1] This is quite likely even when browsing |
@prattmic Thanks! Is there a better way to discuss/resolve this issue - there's no apparent way to directly send feedback to the team looking after the site, nor to get feedback from them on when a fix is rolled out (etc) (More for the future, I think that this particular issue is as resolved as it's going to be) WRT the feedback on the actual issue, I had suspected that the problem might lie in the building of the map for the javascript to know where to link to because of things like tags/commits changing things often (an update every time a PR was merged to the branch would be expensive, I wonder how Git{lab,hub} manage it). |
No, unfortunately I don't think they have much public presence. There should be a "Send feedback" button in the ... menu in the upper right corner of the pages, but that is obviously a one-way mechanism.
FWIW, my experience with GitHub cross-references (not sure about Gitlab) is they seem to be mostly text-search-based which looks like strings that might be the identifier, but is not fully precise. https://cs.opensource.google/ is using Kythe indexing, which actually builds the code to provide completely precise references (for the configuration built; we build linux/amd64 e.g.). The text approach does have the advantage that I imagine it is a bit simpler/cheaper to provide across multiple revisions. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (
go env
)?Chrome/Ubuntu
go env
OutputWhat did you do?
When I go to
https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1792 I click on "QueryRowContext"
What did you expect to see?
I expect to be taken to the defintion of that method
What did you see instead?
I am taken instead to
https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1814
Which is an unrelated call inside an unrelated method
It was pointed out that the definition for
db.QueryRowContext
onmaster
is at https://cs.opensource.google/go/go/+/master:src/database/sql/sql.go;l=1814It appears that the problem is that the javascript is confusing the branches that it is on, it is linking to
The text was updated successfully, but these errors were encountered: