Skip to content

feat(dev): Enhance Makefile for faster go-code with scoping#7736

Merged
dkrotx merged 2 commits intocadence-workflow:masterfrom
dkrotx:dkrot/makefile-with-scoped-go-generate
Feb 24, 2026
Merged

feat(dev): Enhance Makefile for faster go-code with scoping#7736
dkrotx merged 2 commits intocadence-workflow:masterfrom
dkrotx:dkrot/makefile-with-scoped-go-generate

Conversation

@dkrotx
Copy link
Member

@dkrotx dkrotx commented Feb 23, 2026

What changed?
Addressing #7737:

  • Updated the pr target to allow optional scoping for go-generate using the GEN_DIR variable.
  • Modified the go-generate command to reflect the new scope in its output message.

Why?
This change provides developers with the ability to specify a directory scope for code generation, improving flexibility and usability when running code generation tasks.
We extensively use go-generate, and it is slow on the entire cadence
repo, which extends make pr up to 5 mins.
With GEN_DIR=service/matching make pr takes less than a minute.

How did you test it?
No specific tests were added, but existing Makefile targets were verified to ensure they function correctly with the new changes.
make pr GEN_DIR=service/matching # works <1min comparing with the
original 4 mins

Potential risks
No: it's a new optional flag for dev-only Makefile' goal

Release notes

Documentation Changes
Extended make help (or just make w/o goal) to have the example for the "pr" goal.


Reviewer Validation

PR Description Quality (check these before reviewing code):

  • "What changed" provides a clear 1-2 line summary
    • Project Issue is linked
  • "Why" explains the full motivation with sufficient context
  • Testing is documented:
    • Unit test commands are included (with exact go test invocation)
    • Integration test setup/commands included (if integration tests were run)
    • Canary testing details included (if canary was mentioned)
  • Potential risks section is thoughtfully filled out (or legitimately N/A)
  • Release notes included if this completes a user-facing feature
  • Documentation needs are addressed (or noted if uncertain)

@dkrotx dkrotx force-pushed the dkrot/makefile-with-scoped-go-generate branch from d9788ee to e2464df Compare February 23, 2026 11:55
@dkrotx dkrotx changed the title Enhance Makefile for improved code generation scoping feat(dev): Enhance Makefile for faster go-code with scoping Feb 23, 2026
**What changed?**
- Updated the `pr` target to allow optional scoping for `go-generate` using the `GEN_DIR` variable.
- Modified the `go-generate` command to reflect the new scope in its output message.

**Why?**
This change provides developers with the ability to specify a directory scope for code generation, improving flexibility and usability when running code generation tasks.
We extensively use go-generate, and it is slow on the entire cadence
repo, which extends `make pr` up to 5 mins.
With GEN_DIR=service/matching `make pr` takes less than a minute.

**Testing**
No specific tests were added, but existing Makefile targets were verified to ensure they function correctly with the new changes.
make pr GEN_DIR=service/matching # works <1min comparing with the
original 4 mins

**Release notes**
N/A

Signed-off-by: Jan Kisel <dkrot@uber.com>
@dkrotx dkrotx force-pushed the dkrot/makefile-with-scoped-go-generate branch from e2464df to a487b7f Compare February 23, 2026 13:03
@gitar-bot
Copy link

gitar-bot bot commented Feb 24, 2026

Code Review ✅ Approved 1 resolved / 1 findings

Clean, backward-compatible enhancement to scope go generate via GEN_DIR. The self-referencing variable issue from the prior review has been fixed with override and := simple expansion. No new issues found.

✅ 1 resolved
Bug: Self-referencing recursive variable breaks make pr and make go-generate

📄 Makefile:481
Using = (recursively-expanded assignment) for GEN_DIR on line 481 creates a self-referencing variable that causes GNU Make to error when GEN_DIR is not provided:

Makefile:481: *** Recursive variable 'GEN_DIR' references itself (eventually).  Stop.

This breaks the default (no-argument) invocation of both make pr and make go-generate, which is the primary usage path. The release target is also affected since it calls go-generate without GEN_DIR.

I reproduced this locally with make go-generate --dry-run and make pr --dry-run — both fail at line 481.

Additionally, even when GEN_DIR is provided on the command line (e.g., make pr GEN_DIR=service/matching/), the patsubst normalization doesn't take effect because command-line variables override Makefile = assignments, so trailing slashes are not stripped.

Fix: Use override with := (simply-expanded assignment):

override GEN_DIR := $(patsubst %/,%,$(strip $(GEN_DIR)))

This resolves both issues:

  1. No self-reference error when GEN_DIR is empty
  2. patsubst correctly strips trailing slashes even when GEN_DIR is passed on the command line

Verified all three cases work with the fix:

  • make go-generate → uses ./...
  • make go-generate GEN_DIR=service/matching → uses ./service/matching/...
  • make go-generate GEN_DIR=service/matching/ → uses ./service/matching/... (slash stripped) ✓
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@dkrotx dkrotx merged commit 8233466 into cadence-workflow:master Feb 24, 2026
42 checks passed
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.

3 participants