Skip to content

Adjust the library to the latest ProtoData #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jan 14, 2025

Conversation

yevhenii-nadtochii
Copy link
Contributor

@yevhenii-nadtochii yevhenii-nadtochii commented Jan 13, 2025

This PR takes the latest ProtoData and renames FIELD_NAME placeholder to FIELD_PATH.

IfInvalid wrapper has been removed. It was used only to take an error message, but (validate) doesn't have one anymore.

ErrorPlaceholder and RuntimeErrorPlaceholder

These enums are identical. We use a separate version for :java (codegen) module to prevent class clash in runtime of ProtoData. Fat JAR for ProtoData has to have :java-runtime classes inside because they are part of Spine. Otherwise, the app couldn't start. When the test modules in validation try to add the local :java-runtime, we end up with duplicate classes. Changes in the local :java-runtime may cause compilation errors because :java was compiled with the local :java-runtime, but ProtoData CLI uses an older version of the lib. No simple way to filter them out from a fat jar and replace with dependency on the gradle module.

We have considered a separate ProtoData CLI artifact, published specifically for development of :validation. But for this PR, we decided just to bypass the error somehow. Further migration from runtime validation in core-java may drop the necessity to have validation-runtime at all. Implementing a complex solution to a problem, which may disappear soon, is not rational.

@yevhenii-nadtochii yevhenii-nadtochii self-assigned this Jan 13, 2025
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 12.50000% with 14 lines in your changes missing coverage. Please review.

Project coverage is 31.65%. Comparing base (d681c35) to head (6331736).
Report is 11 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #184      +/-   ##
============================================
- Coverage     31.76%   31.65%   -0.12%     
  Complexity      329      329              
============================================
  Files           142      142              
  Lines          2814     2818       +4     
  Branches        229      229              
============================================
- Hits            894      892       -2     
- Misses         1855     1860       +5     
- Partials         65       66       +1     

@yevhenii-nadtochii yevhenii-nadtochii marked this pull request as ready for review January 14, 2025 13:11
@yevhenii-nadtochii
Copy link
Contributor Author

@armiol @alexander-yevsyukov PTAL

@yevhenii-nadtochii yevhenii-nadtochii merged commit 2e6ec50 into master Jan 14, 2025
7 checks passed
@yevhenii-nadtochii yevhenii-nadtochii deleted the take-latest-protodata branch January 14, 2025 13:34
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