Skip to content

Fill in the span property of OneofGroup#346

Merged
yevhenii-nadtochii merged 10 commits intomasterfrom
fix-oneof-span
Apr 14, 2025
Merged

Fill in the span property of OneofGroup#346
yevhenii-nadtochii merged 10 commits intomasterfrom
fix-oneof-span

Conversation

@yevhenii-nadtochii
Copy link
Contributor

@yevhenii-nadtochii yevhenii-nadtochii commented Apr 14, 2025

This PR re-uses the OneofDescriptor.toOneOfGroup() method to create OneofGroup instance. Created this way, the OneofGroup.span field will be correctly populated. Thus, emitted compilation errors will point to the correct file and position.

Other changes are the following:

  1. Added OneofGroup.qualifiedName extension, as we have for Field.
  2. Added TypeName.findJavaClassName() counterpart that returns a nullable result instead of throwing.
  3. Took the latest core-java to pick up the latest changes in spine-time.

@yevhenii-nadtochii yevhenii-nadtochii changed the title Fill in span property of OneOfGroup Fill in the span property of OneOfGroup Apr 14, 2025
@yevhenii-nadtochii yevhenii-nadtochii self-assigned this Apr 14, 2025
@yevhenii-nadtochii yevhenii-nadtochii changed the title Fill in the span property of OneOfGroup Fill in the span property of OneofGroup Apr 14, 2025
@codecov
Copy link

codecov bot commented Apr 14, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 74.81%. Comparing base (db7c00b) to head (703bcbf).
Report is 11 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #346      +/-   ##
============================================
- Coverage     74.87%   74.81%   -0.07%     
  Complexity      615      615              
============================================
  Files           196      196              
  Lines          4259     4256       -3     
  Branches        394      396       +2     
============================================
- Hits           3189     3184       -5     
- Misses          937      939       +2     
  Partials        133      133              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yevhenii-nadtochii yevhenii-nadtochii marked this pull request as ready for review April 14, 2025 12:10
@yevhenii-nadtochii
Copy link
Contributor Author

@armiol @alexander-yevsyukov PTAL

Copy link
Collaborator

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@yevhenii-nadtochii LGTM in general. See my comment though.

val className = javaClassName(header)

/**
* Finds a fully qualified Java class name, generated for the Protobuf type with this name.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure why we would say "fully qualified" here. I would remove.

Copy link
Collaborator

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

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

LGTM

@yevhenii-nadtochii yevhenii-nadtochii merged commit a928b76 into master Apr 14, 2025
8 checks passed
@yevhenii-nadtochii yevhenii-nadtochii deleted the fix-oneof-span branch April 14, 2025 16:16
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