Skip to content

Conversation

artur-ciocanu
Copy link
Contributor

Description

This PRs improves the Protobug codegen by using org.xolstice Maven plugin. It properly detects OS and uses the right protoc binary that doesn't conflict with host binary.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1535

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@artur-ciocanu artur-ciocanu requested review from a team as code owners September 1, 2025 04:08
@artur-ciocanu
Copy link
Contributor Author

@dapr/approvers-java-sdk could you please take a look. This should be an easy review 😄

javier-aliaga
javier-aliaga previously approved these changes Sep 1, 2025
Copy link
Contributor

@javier-aliaga javier-aliaga left a comment

Choose a reason for hiding this comment

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

LGTM

@salaboy
Copy link
Collaborator

salaboy commented Sep 1, 2025

@artur-ciocanu ok.. can you share a bit more how did you tested this? I do like this change.. but the problem that I see is having time to test this..
@javier-aliaga any thoughts about this?

siri-varma
siri-varma previously approved these changes Sep 1, 2025
Copy link
Contributor

@siri-varma siri-varma left a comment

Choose a reason for hiding this comment

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

lgtm

@artur-ciocanu
Copy link
Contributor Author

artur-ciocanu commented Sep 1, 2025

@artur-ciocanu ok.. can you share a bit more how did you tested this? I do like this change.. but the problem that I see is having time to test this.. @javier-aliaga any thoughts about this?

@salaboy great question, the validation process went like this:

  • Generate classes with old Profoc Maven Plugin
  • Genetate classes with new Protobuf Maven Plugin
  • Compare the generated classes and make sure that there are no differences
  • Check that there are no more errors or warning related java-protoc-command not being found
  • Verify that the CI/CD pipeline is successful

Let me know if the validation I have performed are enough or you think we should do some additional checks.

@artur-ciocanu
Copy link
Contributor Author

@salaboy @siri-varma and @javier-aliaga I had to resolve some conflicts, could you please approve one more time.

Thank you!

@javier-aliaga
Copy link
Contributor

@artur-ciocanu reapproved!

@artur-ciocanu artur-ciocanu merged commit e05a744 into dapr:master Sep 2, 2025
10 checks passed
@artur-ciocanu artur-ciocanu deleted the gh-1535 branch September 2, 2025 17:19
Copy link

codecov bot commented Sep 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.88%. Comparing base (d759c53) to head (6bc2776).
⚠️ Report is 216 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1536      +/-   ##
============================================
+ Coverage     76.91%   78.88%   +1.96%     
- Complexity     1592     1935     +343     
============================================
  Files           145      237      +92     
  Lines          4843     5952    +1109     
  Branches        562      611      +49     
============================================
+ Hits           3725     4695     +970     
- Misses          821      938     +117     
- Partials        297      319      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Switch to a newer Protobuf code gen Maven plugin
4 participants