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
feat(iotevents): add DetectorModel L2 Construct #18049
feat(iotevents): add DetectorModel L2 Construct #18049
Changes from 23 commits
e5f18b2
7873d17
07081ed
3d2db07
9c24a19
0723a1c
8f82cbb
fceea45
8ae95cd
ff69d77
d197256
f3511b2
e5baa25
581ddef
50c183c
971d5c4
fbfe028
0b6600d
18e86d0
9744d78
d89d0a0
6ff671e
78fc88c
5506cb5
087bb42
2b4301e
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.
I actually don't like this method. This is actually a
DetectorModel
concern, not aState
concern, that this validation has to be performed.Can we remove this method, and perform this validation in
DetectorModel
by callingtoStateJson()
on itsinitialState
instead?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.
If we implement this validation with
toStateJson()
, It will be as following I think:But we can't get
onEnter?.events
becauseonEnter
can becdk.IResolvable
, same forevents.some()
andevent.condition
.Should
toStateJson()
return more exact type that is compatible withCfnDetectorModel.StateProperty
instead ofCfnDetectorModel.StateProperty
itself?Following is the type
CfnDetectorModel.StateProperty
thattoStateJson()
return: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.
OK, I see the problem. I think we could just check whether
onEnter
is aCfnDetectorModel.OnEnterProperty
, and skip the validation if it's anIResolvable
. But I don't want you to write all of that code if a simpler alternative is possible here.So, let's keep
_toStateJson()
as it is now, but let's make this method@internal
, and rename it to_onEnterEventsHaveAtLeastOneCondition()
.