Skip to content

Change godoc formatting to match Temporal conventions - #95

Open
dplyukhin wants to merge 8 commits into
mainfrom
godoc-formatting
Open

Change godoc formatting to match Temporal conventions#95
dplyukhin wants to merge 8 commits into
mainfrom
godoc-formatting

Conversation

@dplyukhin

@dplyukhin dplyukhin commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What was changed

  • Add newline after the autogenerated comment so it doesn't look like a package doc
  • Capitalize "id" as "ID" instead of "Id" to match the Go SDK
  • Change godoc formatting to match the Go SDK
  • Document signal-with-start defaults in workflow-service.wit (technically this doesn't matter since the source of truth is in temporalio/api)

Why?

To match the Go SDK

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@dplyukhin
dplyukhin requested a review from a team as a code owner July 30, 2026 17:45
@dplyukhin
dplyukhin requested review from bergundy and removed request for bergundy July 30, 2026 17:45
Comment thread advanced/samples/inputs/workflow-service.wit Outdated
- Add newline after the autogenerated comment so it doesn't look like a
  package doc
- Capitalize "id" as "ID" instead of "Id" to match the Go SDK
- Change godoc formatting to match the Go SDK
public string TaskQueue { get; set; }
/// <summary>
/// Total workflow execution timeout, including retries and continue-as-new.
/// Total workflow execution timeout, including retries and continue-as-new. Defaults to unlimited.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Have we confirmed that these defaults are definitely true in every language we're going to generate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did a quick trace before I added the docs and was reasonably confident; in every language we generate, an option being none will become an unset proto field, so the defaults are set by the server. That said, I figured the right place to confirm this type of thing is when we try to merge into the SDKs themselves. That said, this PR isn't really the place to do it anyway because our source of truth is temporalio/api. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't follow that last bit, but if this is common behavior I think it is okay to go in.

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.

2 participants