Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support for creating Custom Event Types with schemas validation #83
base: main
Are you sure you want to change the base?
Support for creating Custom Event Types with schemas validation #83
Changes from 24 commits
ddd02fb
0b345f8
5a2f5fc
0d0c1c0
0f591e6
e32f8c5
51b3ffc
88d3bdd
bbd4279
5309a0f
03f986b
9ff4519
b2c37fd
aab785a
67d2b79
0bb99f2
65856c9
220cb9f
cadb9a7
d737489
c8ffb25
6ce4bad
495c0b3
32846cc
dff52e9
2227b6e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This reads very similarly to the sentence above:
How about changing the wording here to be something more like
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.
This seems really fragile. So any URL that ends with
custom
is assumed to becustom
? Do we define that anywhere? Basically if we have some other even name"some/spec/that-has-custom"
would be skipped is it wasn't a custom eventWhile concerning, and I dont think it needs to be addressed in this PR, can you please add a comment or a TODO to make it less fragile? This may require work in the spec SIG to discuss how to easily identify custom events based on URL.
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.
No, not looking for custom event schema to check
isCustomEvent
here, generating a baseCustomTypeEvent.java
from an official schema.json, which has"$id": "https://cdevents.dev/0.5.0-draft/schema/custom",
.And this class file will be used to create any custom events
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.
We discussed this in the CDEvents WG and agreed that extending the match to
schema/custom
should be sufficient. WDYT?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.
yeah sounds good to me.
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.
nit: We can probably just this up some and inverse the if statement