Repository navigation
Add DataSchema parameter to CloudEvent creation and data setters - #59
Conversation
Co-authored-by: duglin <1944671+duglin@users.noreply.github.com>
|
@oising look ok to you? |
|
@copilot please fix the CI issues |
Co-authored-by: duglin <1944671+duglin@users.noreply.github.com>
Looking... It seems there is no binding-by-property-name configured on the parameter. I'm looking at the suite as a whole to see if this was ignored en-masse |
Lgtm Room for improvement but not in this PR |
|
You sure? We could ask copilot to improve it |
Well okay, hold off until tomorrow and I'll give you some hints for the robot |
|
@oising merge or wait? |
Reviewing now... |
I went through the current design with copilot and instructed it on how to have better cohesion between commands as well as improving the ux/dx, and I came up with this plan -- it wouldn't strictly be breaking, but I would be inclined to bump a minor version at least: Plan: Pipeline binding improvements for CloudEvents.SdkCurrent state (
|
|
@oising yes different PR, but I also am not sure I'm qualified to agree (or disagree) with the plan. My goal in getting involved here was to see if I can help unblock some easy issues/PR with the help of copilot. Adding brand new features w/o me really understanding the SDK is kind of risky (and i don't have the time to deep dive into it). If I'm following, the analysis identified a couple of bugs in the current stuff, and those seem like things that are worth looking into even w/o the rest of the stuff. Would you agree? Checkout: #58 - do you agree with the sentiment that the need for the SDK is minimal (at best)? My current thinking... get copilot to fix the existing bugs you identified and then wait for some signs of life before we do a ux/dx change. WDYT? |
|
To be clear, the two bugs are:
|
Yes, agree.
It's hard to tell - in its current form, it's already a fairly simple SDK that doesn't add a great deal of utility, IMO. It also doesn't follow a lot of the conventions for making functions/cmdlets composable: i.e. you should be able to pipe the output of one directly or indirectly into another without having to manually construct intermediate, temporary objects to compensate for mismatched or missing property/type bindings, assuming they cover the same surface area/functionality. An example of this would be how you can do the following: ps> dir foo.txt | cat The reason this works is because cat (a.k.a. get-content has a -LiteralPath property that binds by-name to the Path property on incoming piped objects.)
Yes to fixing the current bugs. As for waiting, we could, but we might be in a chicken and egg situation. Maybe nobody uses it because it doesn't get much love and/or isn't very composable (as it stands.) I am happy to put some work into bringing it up to date. I am a ten-year powershell MVP (got some work in books, https://www.amazon.ca/PowerShell-Deep-Dives-Jeffery-Hicks/dp/1617291315 ) and work in distributed systems design, so I'm very familiar with cloudevents and related things. |
|
@oising if you're willing to take on the task of bringing it up to where you think it needs to be, then that would be great. Obviously, it would be best to not break existing users, but we can always discuss that as we go along. For now I'll merge this PR and open an issue for the bugs and the proposed changes in: #59 (comment) |
New-CloudEvent,Set-CloudEventData, andSet-CloudEventJsonDatalacked a parameter for setting the CloudEventsdataschemaattribute.-DataSchemato all three cmdlets, accepting an absolute URI.