feat(iotevents): support comparison operators #19329

yamatatsu · 2022-03-10T14:26:26Z

This PR includes to implement comparison operators.

integ test

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-03-10T14:26:28Z

skinny85

Looks good @yamatatsu! One question.

packages/@aws-cdk/aws-iotevents/lib/expression.ts

Pull request has been modified.

yamatatsu · 2022-03-11T15:11:23Z

packages/@aws-cdk/aws-iotevents/test/integ.detector-model.ts

+      onInput: [{
+        eventName: 'test-input-event',
+        actions: [
+          ['neq', iotevents.Expression.neq(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['gt', iotevents.Expression.gt(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['lt', iotevents.Expression.lt(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['gte', iotevents.Expression.gte(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['lte', iotevents.Expression.lte(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['or', iotevents.Expression.or(iotevents.Expression.fromString('true'), iotevents.Expression.fromString('false'))] as const,
+        ].map(([variableName, value]) => ({ bind: () => ({ configuration: { setVariable: { variableName, value: value.evaluate() } } }) })),
+      }],


@skinny85
I have added integ test but it is efficient?
I wanted to confirm that these expressions would work on AWS actually. And its purpose is filled. But I wonder this integ code have value for aws-cdk.
WDYT?

Yeah, I think I would skip this. Doesn't seem particularly elegant.

I could see doing this in a unit test, but I wouldn't do it an an integ test.

skinny85 · 2022-03-12T02:22:49Z

@yamatatsu note that I'll be on vacation for the next 2 weeks, so my responses here might be delayed. Thanks for your patience!

yamatatsu · 2022-04-02T13:43:49Z

packages/@aws-cdk/aws-iotevents/lib/expression.ts

+    /**
+     * Indicates the priority of the operator.
+     * This is intended to be set according to MDN rules.
+     * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#table


I have adopted MDN as the operator priority rule. I think it can be used to whatever follow general rules and has concrete numbers.

skinny85

Looks fantastic @yamatatsu! Super clean, and doesn't render the extra parenthesis. Thanks for the extra effort you put into this!

I have a few last stylistic comments, but other than that, we're good to go here!

skinny85 · 2022-04-04T23:37:10Z

packages/@aws-cdk/aws-iotevents/test/integ.detector-model.ts

+      onInput: [{
+        eventName: 'test-input-event',
+        actions: [
+          ['neq', iotevents.Expression.neq(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['gt', iotevents.Expression.gt(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['lt', iotevents.Expression.lt(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['gte', iotevents.Expression.gte(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['lte', iotevents.Expression.lte(iotevents.Expression.fromString('31.7'), iotevents.Expression.fromString('31.7'))] as const,
+          ['or', iotevents.Expression.or(iotevents.Expression.fromString('true'), iotevents.Expression.fromString('false'))] as const,
+        ].map(([variableName, value]) => ({ bind: () => ({ configuration: { setVariable: { variableName, value: value.evaluate() } } }) })),
+      }],


Yeah, I think I would skip this. Doesn't seem particularly elegant.

I could see doing this in a unit test, but I wouldn't do it an an integ test.

packages/@aws-cdk/aws-iotevents/lib/expression.ts

packages/@aws-cdk/aws-iotevents/test/detector-model.test.ts

packages/@aws-cdk/aws-iotevents/lib/expression.ts

Pull request has been modified.

skinny85

Thanks for the contribution @yamatatsu!

mergify · 2022-04-05T16:53:31Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2022-04-07T23:48:19Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Pull request has been modified.

mergify · 2022-04-08T18:07:34Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-04-08T18:51:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: a5737a8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-04-08T18:52:17Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

This PR includes to implement comparison operators. - [x] integ test ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

This PR adds rest binary operators of IoT Events. This PR does not add new integ tests because of [this conversation](#19329 (comment)). ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

This PR adds rest binary operators of IoT Events. This PR does not add new integ tests because of [this conversation](aws#19329 (comment)). ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

feat(iot-events): support comparison operators

db278a4

github-actions bot added the @aws-cdk/aws-iotevents Related to AWS IoT Events label Mar 10, 2022

github-actions bot assigned skinny85 Mar 10, 2022

yamatatsu marked this pull request as draft March 10, 2022 16:12

skinny85 previously requested changes Mar 11, 2022

View reviewed changes

packages/@aws-cdk/aws-iotevents/lib/expression.ts Outdated Show resolved Hide resolved

add integ test

50a3721

yamatatsu commented Mar 11, 2022

View reviewed changes

yamatatsu marked this pull request as ready for review March 11, 2022 20:31

yamatatsu requested a review from skinny85 March 11, 2022 20:31

yamatatsu added 2 commits April 2, 2022 20:45

Merge branch 'master' into iotevents-comparison-operator

da51c45

add priority of the operators

9d59b18

yamatatsu commented Apr 2, 2022

View reviewed changes

skinny85 previously requested changes Apr 5, 2022

View reviewed changes

Merge branch 'master' into iotevents-comparison-operator

ec4656c

address comments

f8e25e9

yamatatsu requested a review from skinny85 April 5, 2022 12:44

skinny85 added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Apr 5, 2022

skinny85 previously approved these changes Apr 5, 2022

View reviewed changes

skinny85 changed the title ~~feat(iot-events): support comparison operators~~ feat(iotevents): support comparison operators Apr 5, 2022

github-actions bot added the p2 label Apr 5, 2022

Merge branch 'master' into iotevents-comparison-operator

c61d8cd

Merge branch 'master' into iotevents-comparison-operator

223688a

skinny85 approved these changes Apr 8, 2022

View reviewed changes

Merge branch 'master' into iotevents-comparison-operator

a5737a8

mergify bot merged commit 95cb3f3 into aws:master Apr 8, 2022

yamatatsu mentioned this pull request Apr 13, 2022

chore(iot-events): support all binary operators #19889

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iotevents): support comparison operators #19329

feat(iotevents): support comparison operators #19329

yamatatsu commented Mar 10, 2022 •

edited

Loading

gitpod-io bot commented Mar 10, 2022

skinny85 left a comment

yamatatsu Mar 11, 2022

skinny85 Apr 4, 2022

skinny85 commented Mar 12, 2022

yamatatsu Apr 2, 2022

skinny85 left a comment

skinny85 Apr 4, 2022

skinny85 left a comment

mergify bot commented Apr 5, 2022

mergify bot commented Apr 7, 2022

mergify bot commented Apr 8, 2022

aws-cdk-automation commented Apr 8, 2022

mergify bot commented Apr 8, 2022

feat(iotevents): support comparison operators #19329

feat(iotevents): support comparison operators #19329

Conversation

yamatatsu commented Mar 10, 2022 • edited Loading

gitpod-io bot commented Mar 10, 2022

skinny85 left a comment

Choose a reason for hiding this comment

yamatatsu Mar 11, 2022

Choose a reason for hiding this comment

skinny85 Apr 4, 2022

Choose a reason for hiding this comment

skinny85 commented Mar 12, 2022

yamatatsu Apr 2, 2022

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 Apr 4, 2022

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Apr 5, 2022

mergify bot commented Apr 7, 2022

mergify bot commented Apr 8, 2022

aws-cdk-automation commented Apr 8, 2022

AWS CodeBuild CI Report

mergify bot commented Apr 8, 2022

yamatatsu commented Mar 10, 2022 •

edited

Loading