fix: return the sdkVersion to ModelBuildInfo - #2547
Conversation
There was a problem hiding this comment.
🟢 Ready to approve
The change is narrowly scoped to restoring a previously expected metadata field and all in-repo instantiations were updated consistently.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Restores the sdkVersion field in ModelBuildInfo and ensures it is emitted into timefold/build-info.json during Quarkus build-time processing, re-establishing compatibility with downstream platform consumers that rely on this metadata.
Changes:
- Reintroduced
sdkVersioninto theModelBuildInforecord (including updatingempty()). - Updated Quarkus deployment build-step to populate
sdkVersion(currently set to the same value assolverVersion) when generatingbuild-info.json.
File summaries
| File | Description |
|---|---|
| service/quarkus/deployment/src/main/java/ai/timefold/solver/service/quarkus/deployment/TimefoldModelDescriptorProcessor.java | Writes build-info.json with the restored sdkVersion field populated at build time. |
| service/definition/src/main/java/ai/timefold/solver/service/definition/internal/descriptor/ModelBuildInfo.java | Adds sdkVersion back to the metadata record and updates the empty() factory accordingly. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
d4a98b5 to
c36ec8c
Compare
There was a problem hiding this comment.
🟢 Ready to approve
The change is small, locally contained, and preserves the previous constructor signature while restoring the missing field.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
The EMPTY constant currently calls a non-existent 5-arg constructor and will fail compilation after adding the sixth record component.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟢 Ready to approve
The change cleanly restores the expected field while preserving the previous constructor signature and aligns with existing Quarkus record patterns in the module.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Putting back the SDK version since it breaks the platform. Removal needs to be properly planned.