Skip to content
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

docs: Explain Frame.Module and Frame.Package #385

Merged
merged 1 commit into from Oct 11, 2021

Conversation

rhcarvalho
Copy link
Contributor

The naming can be confusing since "Go Package" and "Go Module" have a very specific meaning in Go, but that meaning doesn't match the platform-agnostic meaning from the Sentry protocol.

See also #377 (comment).

Comment on lines +163 to +167
Function string `json:"function,omitempty"`
Symbol string `json:"symbol,omitempty"`
// Module is, despite the name, the Sentry protocol equivalent of a Go
// package's import path.
Module string `json:"module,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the same formatting? See the types of the items below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go code is formatted in a standard way with a tool, gofmt. This is the output of the tool.

The tool aligns fieldname-type-structtag for contiguous blocks of fields, that's what you see below and before adding the documentation.

"Documented" fields lead to no block alignment, for example see this in the Go standard library:

https://cs.opensource.google/go/go/+/refs/tags/go1.17.2:src/net/http/request.go;l=97-324

@rhcarvalho rhcarvalho merged commit 2a68520 into master Oct 11, 2021
@rhcarvalho rhcarvalho deleted the docs-frame-module-package branch October 11, 2021 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants