chore: add deployment_id to ProjectUpdate proto#1942
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThe PR removes the Publish RPC and all related plumbing (client interface method, gRPC/Connect client wrapper, and CLI SendMsg) and deletes the PublishRequest and Event protobuf messages. Separately, it adds a new exported field Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/protos/io/defang/v1/fabric.proto (2)
632-646: Aligndeployment_idnaming with existingetagfields.Most messages use
etagfor the deployment identifier, whileProjectUpdateintroducesdeployment_id. Consider standardizing the field name for API consistency (e.g., useetaghere or rename others before release).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/protos/io/defang/v1/fabric.proto` around lines 632 - 646, ProjectUpdate defines the deployment identifier as deployment_id which is inconsistent with the rest of the API that uses etag; change the field name in the ProjectUpdate message from deployment_id to etag (keep the same field number and type: string = 13) and update all references/usages and generated code accordingly so consumers still receive the same wire-format but use the standardized field name (also search for ProjectUpdate and any serializers/deserializers to rename usages).
238-247: Naming inconsistency: consider usingetagconsistently across all messages.Most messages use
etagfor the deployment identifier (DeployResponse, ServiceInfo, TailRequest, LogEntry, TailResponse, PreviewRequest, PreviewResponse), but ProjectUpdate usesdeployment_idat tag 13. Both include comments indicating they refer to the same concept. Standardizing on a single field name would improve API consistency.All tag numbers are assigned sequentially without reuse or conflicts—no breaking compatibility issues detected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/protos/io/defang/v1/fabric.proto` around lines 238 - 247, ProjectUpdate uses the field deployment_id (tag 13) while other messages use etag for the same deployment identifier; update ProjectUpdate to use the same field name etag (replacing deployment_id) and preserve tag 13 and its comment, ensuring consistency with messages like DeployResponse, ServiceInfo, TailRequest, LogEntry, TailResponse, PreviewRequest, and PreviewResponse; adjust any generated code/refs that reference ProjectUpdate.deployment_id to ProjectUpdate.etag to keep the API consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/protos/io/defang/v1/fabric.proto`:
- Around line 632-646: ProjectUpdate defines the deployment identifier as
deployment_id which is inconsistent with the rest of the API that uses etag;
change the field name in the ProjectUpdate message from deployment_id to etag
(keep the same field number and type: string = 13) and update all
references/usages and generated code accordingly so consumers still receive the
same wire-format but use the standardized field name (also search for
ProjectUpdate and any serializers/deserializers to rename usages).
- Around line 238-247: ProjectUpdate uses the field deployment_id (tag 13) while
other messages use etag for the same deployment identifier; update ProjectUpdate
to use the same field name etag (replacing deployment_id) and preserve tag 13
and its comment, ensuring consistency with messages like DeployResponse,
ServiceInfo, TailRequest, LogEntry, TailResponse, PreviewRequest, and
PreviewResponse; adjust any generated code/refs that reference
ProjectUpdate.deployment_id to ProjectUpdate.etag to keep the API consistent.
Description
EventmessagePublishrpcLinked Issues
Checklist
Summary by CodeRabbit
Removed Features
API Changes