Skip to content

Conversation

@yordis
Copy link
Member

@yordis yordis commented Nov 7, 2025

No description provided.

@cursor
Copy link

cursor bot commented Nov 7, 2025

PR Summary

Adds an --http_transcode option to generate servers with HTTP/JSON transcoding and updates docs and tests accordingly.

  • Generator (cmd/protoc-gen-elixir-grpc/main.go):
    • Add --http_transcode flag; parse and propagate into generation.
    • When enabled, generated modules use GRPC.Server, service: ..., http_transcode: true.
    • Update help usage text.
  • Tests (cmd/protoc-gen-elixir-grpc/main_test.go):
    • Add cases for http_transcode=true and combined with handler_module_prefix.
    • Adjust expected outputs to include http_transcode: true when set.
  • Docs (cmd/protoc-gen-elixir-grpc/README.md):
    • Document configuration options with examples for http_transcode and handler_module_prefix, including combined usage.

Written by Cursor Bugbot for commit c5dce2f. This will update automatically on new commits. Configure here.

@coderabbitai
Copy link

coderabbitai bot commented Nov 7, 2025

Walkthrough

This pull request adds HTTP transcoding support to the protoc-gen-elixir-grpc plugin through a new command-line flag, updates function signatures to thread the flag through the generation workflow, adds conditional logic to the generated server module, and extends test coverage with documentation.

Changes

Cohort / File(s) Summary
Documentation
cmd/protoc-gen-elixir-grpc/README.md
Added new documentation sections covering HTTP Transcoding configuration, Custom Handler Module Prefix, and examples for combining options.
Implementation
cmd/protoc-gen-elixir-grpc/main.go
Introduced http_transcode flag and constant for command-line configuration. Updated generateElixirFile() and generateServiceModule() function signatures to accept httpTranscode boolean parameter. Modified server module generation to conditionally append http_transcode: true to GRPC.Server use expression when flag is enabled. Thread-safe propagation of flag through generation workflow.
Test Coverage
cmd/protoc-gen-elixir-grpc/main_test.go
Added test scenarios validating HTTP transcoding flag alone and in combination with handler module prefix options. New tests verify generated file names and server module structure with http_transcode configuration applied.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Areas requiring attention:
    • Verify that the httpTranscode flag is correctly propagated through all call sites in main.go
    • Confirm the conditional logic for appending http_transcode: true to the server module output is correct
    • Review test assertions in main_test.go to ensure they match the expected generated code format

Possibly related PRs

Poem

🐰✨ A flag hops through the code so neat,
HTTP transcoding, a feature complete!
The server now dances with options so bright,
Thread-safe and tested—everything right! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to evaluate relatedness to the changeset. Add a pull request description explaining the purpose of HTTP transcoding support, how it works, and why these changes are needed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: adding HTTP transcoding support to the protoc-gen-elixir-grpc tool and updating corresponding tests.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch yordis/feat-server-opts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yordis yordis marked this pull request as ready for review November 7, 2025 19:43
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis yordis force-pushed the yordis/feat-server-opts branch from d74668d to c5dce2f Compare November 7, 2025 19:44
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
cmd/protoc-gen-elixir-grpc/main_test.go (1)

328-377: Good coverage of combined options.

This test validates that http_transcode and handler_module_prefix work together correctly, which is important for real-world usage.

Consider adding a test for http_transcode + package_prefix combination for complete coverage, though the current tests provide good confidence in the implementation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10d189c and c5dce2f.

📒 Files selected for processing (3)
  • cmd/protoc-gen-elixir-grpc/README.md (1 hunks)
  • cmd/protoc-gen-elixir-grpc/main.go (6 hunks)
  • cmd/protoc-gen-elixir-grpc/main_test.go (5 hunks)
🔇 Additional comments (9)
cmd/protoc-gen-elixir-grpc/README.md (1)

38-96: Excellent documentation structure!

The new configuration sections are well-organized and provide clear examples for users. The progression from individual options to combined usage is helpful, and the generated code samples match the test expectations.

cmd/protoc-gen-elixir-grpc/main_test.go (1)

175-224: Good test coverage for http_transcode flag.

The test correctly validates the new flag behavior, checking both the generated file path and the presence of http_transcode: true in the server module.

cmd/protoc-gen-elixir-grpc/main.go (7)

56-58: Clean constant definition and usage update.

The httpTranscodeFlag constant follows the existing naming pattern. The usage string is quite long but clearly documents the available flags.


115-119: Proper flag declaration.

The boolean flag is correctly declared with a sensible default (false) that maintains backwards compatibility.


171-171: Parameter correctly threaded through generation workflow.

The httpTranscode flag is properly dereferenced and passed to generateElixirFile.


186-186: Function signature updated correctly.

The signature change adds the httpTranscode parameter and maintains consistency with the call site.


200-200: Parameter forwarded to service generation.

The flag is correctly passed through to generateServiceModule.


210-210: Function signature extended appropriately.

The signature accepts the httpTranscode parameter needed for conditional output generation.


215-219: Clean conditional output generation.

The logic correctly appends , http_transcode: true to the use GRPC.Server statement when the flag is enabled. The implementation produces output that matches the test expectations and maintains proper formatting.

@yordis yordis merged commit 2ab8496 into main Nov 7, 2025
4 checks passed
@yordis yordis deleted the yordis/feat-server-opts branch November 7, 2025 19:49
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