Repository navigation
Feature: Serialization context tests - #868
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69da1abdf9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
68fc88c to
989fab2
Compare
|
PHP part is in separate PR: xepozz#1 |
|
Generally looks good! Other then the codex comments and failing CI jobs |
Register child_workflow_payloads_default_id in PreparedFeature.ALL. Drop the go minVersion gate: features.go imports every feature, so it never guarded compilation. Drop the Go local_activity_payloads until sdk-go#2562 is released.
The marshaller drops repeated fields coming from the protobuf C extension, which the runtime image loads, so schedule describe fails on the released SDK. Pin the branch that carries the fix and stop passing an explicit PHP version, which would override the pin.
The branch carrying the marshaller and serialization context fixes is serialization-context2, not feature/serialization-context2.
php-ver was built as 'v' + the version, so the empty version the pin needs produced "v" and build-image rejected it as invalid semver. The job takes no repo ref, so there is no release to build an image from until the fix ships.
|
@Quinn-With-Two-Ns all green. I think I'll release a new minor/patch after serialization context merged and revert composer/ci changes |
|
@Quinn-With-Two-Ns please review again and let's proceed it further |
|
Codex comments still not addressed, if they have been resolved please comment on them with the resolution and close them or address them, or explain why they are not real issues |
temporalio/sdk-go#2562 shipped in v1.49.0.
|
@Quinn-With-Two-Ns Codex comments resolved, PTAL. |
2 participants
What was changed
Why?
Implementing Serialization Context for PHP, need parity tests
Checklist
Closes
How was this tested: