Skip to content

Conversation

alexshtin
Copy link
Contributor

@alexshtin alexshtin commented May 8, 2025

What changed?

Refactor code generators:

  1. Extracted templates to separate files.
  2. Extracted common helpers to codegen package.
  3. Simplified code generators.

All generated files are almost unchanged (removed leading empty line).

Why?

Better maintainability.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@alexshtin alexshtin requested a review from a team as a code owner May 8, 2025 08:52
@alexshtin alexshtin requested a review from dnr May 8, 2025 08:52
@@ -0,0 +1,167 @@
{{- /*gotype: go.temporal.io/server/cmd/tools/gendynamicconfig.dynamicConfigData*/ -}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line enables intellisense in GoLand.

Copy link
Contributor

Choose a reason for hiding this comment

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

ooh, very cool. I'm jealous of that, the template has been frustrating to edit

func generateFrontendOrAdminClient(w io.Writer, service service) {
writeTemplatedCode(w, service, `
func generateFrontendOrAdminClient(w io.Writer, service service) error {
writeTemplatedCode(w, service, `// Code generated by cmd/tools/genrpcwrappers. DO NOT EDIT.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All templates here also can be extracted to separate files, but it is too much work. Maybe later.

Copy link
Contributor

@dnr dnr left a comment

Choose a reason for hiding this comment

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

awesome, this is much nicer!

Comment on lines +25 to +31
{{end -}}
}
{{- end}}
{{- else}}
return nil
{{- end -}}
{{- end}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope it didn't take that long to get all the {{- and -}} correct 😳

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I mostly just copied it from what you had. But yes, spent some time on it :-)

{{ if $T.IsGeneric -}}
type {{$P.Name}}TypedSetting[T any] setting[T, func({{$P.GoArgs}})]

// New{{$P.Name}}TypedSetting creates a setting that uses mapstructure to handle complex structured
Copy link
Contributor

Choose a reason for hiding this comment

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

aw, this is going to conflict with 7052 that's almost ready. I'll handle the conflicts

@@ -0,0 +1,167 @@
{{- /*gotype: go.temporal.io/server/cmd/tools/gendynamicconfig.dynamicConfigData*/ -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

ooh, very cool. I'm jealous of that, the template has been frustrating to edit

@alexshtin alexshtin enabled auto-merge (squash) May 9, 2025 07:09
@alexshtin alexshtin force-pushed the refactor/codegen branch from 63c318e to e8c8dad Compare May 9, 2025 07:17
@alexshtin alexshtin merged commit 5d9f203 into temporalio:main May 9, 2025
52 checks passed
@alexshtin alexshtin deleted the refactor/codegen branch May 9, 2025 07:58
josesa added a commit to josesa/temporal that referenced this pull request May 12, 2025
* main: (22 commits)
  Add host health metrics gauge (temporalio#7728)
  add rule expiration check (temporalio#7749)
  Add activity options to the pending activity info (temporalio#7727)
  Enable DLQ V2 for replication (temporalio#7746)
  chore: be smarter about when to use Stringer vs String (temporalio#7743)
  versioning entity workflows: enabling auto-restart pt1 (temporalio#7715)
  Refactor code generators (temporalio#7734)
  allow passive to generate replication tasks (temporalio#7713)
  Validate links in completion callbacks (temporalio#7726)
  CHASM: Engine Update/ReadComponent implementation (temporalio#7696)
  Enable transition history in dev env and tests (temporalio#7737)
  chore: Add Stringer tags (temporalio#7738)
  Add internal pod health check to DeepHealthCheck (temporalio#7709)
  Rename internal CHASM task processing interface (temporalio#7730)
  [Frontend] Log slow gRPC requests (temporalio#7718)
  Remove cap for dynamic config callback pool (temporalio#7723)
  Refactor updateworkflowoptions package (temporalio#7725)
  Remove a bunch of redundant utf-8 validation (temporalio#7720)
  [CHASM] Pure task processing - GetPureTasks, ExecutePureTasks (temporalio#7701)
  Send ActivityReset flag to the worker in heartbeat response (temporalio#7677)
  ...
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