Update Godoc for internal structs/funcs to clarify the package they are exposed under #1735
Conversation
yuandrew
force-pushed
the
godoc-internal-clarity
branch
from
882b677 to
e1c3991
Compare
internal/activity.go
Outdated
| @@ -39,11 +39,15 @@ import ( | |||
|
|
|||
| type ( | |||
| // ActivityType identifies an activity type. | |||
| // | |||
| // Exposed as: activity:Type | |||
There was a problem hiding this comment.
hm Can we make these doc links https://tip.golang.org/doc/comment#doclinks?
There was a problem hiding this comment.
That way they are navigable
There was a problem hiding this comment.
yeah, good idea. Unfortunately it's not as clean because we have to import the entire package name, i.e. go.temporal.io/sdk/temporal.ApplicationError, but clickable is better
internal/error.go
Outdated
| will return original type of error represented as string. Workflow code could check this type to determine what kind of error it was | ||
| and take actions based on the type. These errors are retryable by default, unless error type is specified in retry policy. | ||
| *ApplicationError can be returned in two cases: | ||
| - If activity implementation returns *ApplicationError by using NewApplicationError()/NewNonRetryableApplicationError() API. |
There was a problem hiding this comment.
There appears to be some unnecessary whitespace changes
There was a problem hiding this comment.
oops, this was because I was stripping whitespace at the beginning and ending of lines, and format.Source([]byte(newFile)) won't automatically add back spacing to comments. Fixed!
| return publicToInternal, nil | ||
| } | ||
|
|
||
| func isPublic(name string) bool { |
There was a problem hiding this comment.
| log.Fatal(err) | ||
| } | ||
| if missing { | ||
| log.Fatal("Missing documentation, see previous stdout for which objects. Re-run command with -fix to auto-generate missing docs.") |
There was a problem hiding this comment.
This is great that it gives a bad exit code on missing docs. Can you add this step to CI?
There was a problem hiding this comment.
yep! This was actually added to the check command https://github.com/temporalio/sdk-go/pull/1735/files#diff-265865b1cb4e06ecf08b23b205f709fb4281ba4d4804cbe46f8b91a725073fd5
What was changed
Added a new tool that identifies public structs/funcs that map to internal fields, and detects if the internal fields have a doc comment referring to where it's publicly exposed as.
Passing by with no flags will only output missing mappings to stdout, but passing in
-fixwill automatically generate the mapping comments.Why?
Clarity
Checklist
Closes Update Godoc for
internalstructs/funcs to clarify the package they are exposed under #1681How was this tested:
Manually inspected output