-
Notifications
You must be signed in to change notification settings - Fork 16
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
Replace custom CloudEvent implementation in favor of official CloudEvent SDK #129
base: main
Are you sure you want to change the base?
Conversation
6345314
to
cea9fef
Compare
Codecov Report
@@ Coverage Diff @@
## main #129 +/- ##
============================================
- Coverage 73.48% 73.44% -0.04%
- Complexity 562 565 +3
============================================
Files 72 72
Lines 1746 1766 +20
============================================
+ Hits 1283 1297 +14
- Misses 463 469 +6
Continue to review full report at Codecov.
|
LGTM, I'll fix the integration tests unless you want to take a stab at it (looks like an issue with Laravel). |
Sure thing, I'll take a look |
Seemed to be an issue with the locked version of laravel/sail's 8.0 runtime. Simply getting a new lock file should do the trick 🤞 |
@CiiDyR is this PR ready to be merged? |
Yep, should be good to go, same with #127 |
Thanks @CiiDyR! I have no idea what the DCO action is or why it is failing. I think I have to do something special, but I have no idea what nor do I currently have access to any chat channels to ask silly questions. I'll make a point to figure it out this week. But yeah, this can be merged as soon as I figure out this mysterious action that I didn't add to the repo means. |
8dd204b
to
9473f3f
Compare
I think I fixed it by amending the commits like described here: https://github.com/src-d/guide/blob/master/developer-community/fix-DCO.md So should be ready for merging now, I guess at least... |
Awesome! I'll get this merged this evening! |
@CiiDyR, I've updated some bits, would you mind rebasing on the latest |
446c25b
to
d17e37e
Compare
Signed-off-by: Hendrik Heil <[email protected]>
ff09250
to
0ea3176
Compare
Signed-off-by: Hendrik Heil <[email protected]>
Signed-off-by: Hendrik Heil <[email protected]>
Signed-off-by: Hendrik Heil <[email protected]>
Signed-off-by: Hendrik Heil <[email protected]>
Signed-off-by: Hendrik Heil <[email protected]>
For some reason the rebase duplicated the commits, so in order to keep the history clean, I just made new commits. |
Signed-off-by: Hendrik Heil <[email protected]>
@withinboredom As far as I can tell the jobs fail because |
Signed-off-by: Hendrik Heil <[email protected]>
Thanks @CiiDyR! Sorry for the delay, my computer died a horrible death after 4 years of fantastic service. I've just got my new computer all set up and I'll be getting to this sometime in the next week. Thanks again for your contribution here! |
Any updates on this PR? :) |
Sorry @CiiDyR, I'll get to this soon! |
In exactly 2 weeks, I'll be going on a sabbatical from regular work for 3 months. Until then, things are just too crazy to get this merged. Once I'm on my sabbatical, I plan to focus on this and a couple of other open-source projects. I just wanted to give you a heads up @CiiDyR. I haven't forgotten about you or this PR. |
Description
This PR aims to add support for the official CloudEvent PHP SDK while deprecating the use of the current CloudEvent implementation.
Issue reference
#128
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: