-
Notifications
You must be signed in to change notification settings - Fork 50
Make Events class Serializable #282
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
Make Events class Serializable #282
Conversation
Signed-off-by: Vishesh Ruparelia <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fjtirado didn't you work on something similar?
@ricardozanini @tsurdilo would it be possible to release this change as a part of the upcoming release? |
@visheshruparelia I'll release 4.0.5 first, and then we can add this commit for a follow-up 4.0.6. I didn't want to change the PR now since ppl already tested it. @fjtirado can you also review? |
Thanks @ricardozanini for that. |
We just got the ball rolling back again. Since we don't have a spec 0.9, I'm prioritizing CVEs and bugs on 0.8. Releases should be at a quick pace now since I won't be cherry-picking many PRs for each patch release. Having said that, we release it as soon as necessary. You can ping me anytime on slack to release. Next one, all the bumps from dependabot + this one probably. |
Alright, thank you!
…On Wed, 25 Oct 2023 at 21:57, Ricardo Zanini ***@***.***> wrote:
Thanks @ricardozanini <https://github.com/ricardozanini> for that. Just
curious on the release cycle for this repo. When is the targeted release
for 4.0.6?
We just got the ball rolling back again. Since we don't have a spec 0.9,
I'm prioritizing CVEs and bugs on 0.8. Releases should be at a quick pace
now since I won't be cherry-picking many PRs for each patch release. Having
said that, we release it as soon as necessary. You can ping me anytime on
slack to release. Next one, all the bumps from dependabot + this one
probably.
—
Reply to this email directly, view it on GitHub
<#282 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFSBP73K3BDLW3SOMXA7BUDYBE4WFAVCNFSM6AAAAAA6PU23ASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZZGY2DCOJUHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ricardozanini good for me |
@visheshruparelia you can test the main branch on your end. |
Sure, by testing you mean evaluate if this fix solved my usecase or do you
mean some sort of regression test?
…On Thu, 26 Oct 2023 at 18:01, Ricardo Zanini ***@***.***> wrote:
@visheshruparelia <https://github.com/visheshruparelia> you can test the
main branch on your end.
—
Reply to this email directly, view it on GitHub
<#282 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFSBP74XGZIZUWBZ2BTO4TDYBJJZNAVCNFSM6AAAAAA6PU23ASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBRGAZDSMBWGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Both would be great! And you can report it back here, please? |
Sure, will get back. |
@ricardozanini I tested in my local and turns out there were some more classes which needed to be serializable. |
We can continue the discussion in that PR if you don't mind. Thanks! |
What this PR does / why we need it:
It makes the Events class Serialiazable. Other classes are already implementing this interface so making this one consistent with others.
Special notes for reviewers:
Corresponding discussion on Slack: https://cloud-native.slack.com/archives/CMBAZQSEP/p1692019690656669