Skip to content
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 Message Operator #247

Merged
merged 3 commits into from Jun 23, 2021
Merged

Support for Message Operator #247

merged 3 commits into from Jun 23, 2021

Conversation

cmoesel
Copy link
Member

@cmoesel cmoesel commented Jun 23, 2021

This PR adds support for user-provided MessageListeners that are invoked when a CQL Message operator should log a message. For more info see the spec for Message and the new documentation in Overview.md.

Submitter:

  • This pull request describes why these changes were made
  • Code diff has been done and been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
  • Tests are included and test edge cases
  • Tests have been run locally and pass
  • Code coverage has not gone down and all code touched or added is covered.
  • Code passes lint and prettier (hint: use yarn run test:plus to run tests, lint, and prettier)
  • All dependent libraries are appropriately updated or have a corresponding PR related to this change
  • cql4browsers.js built with yarn run build:browserify if source changed.

Reviewer:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

Also provide a built-in NullMessageListener and ConsoleMessageListener.
@kjmahalingam kjmahalingam self-requested a review June 23, 2021 13:28
OVERVIEW.md Outdated
// do something with the message
}
```
The `source` argument may be of any type (depending on where the `Message` operator is used in the CQL), but the `code`, `severity`, and `message` arguments are all strings. According the the specification, the `source` argument is supplied for messages w/ Trace severity and implementers should take care to ensure that no PII or PHI is logged as part of the trace message. The CQL execution framework does not redact any PII/PHI, so it is up to the implementer of the MessageListener to ensure appropriate precautions are taken.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a typo; should be "According to the specification" but there's a double "the"

@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2021

Codecov Report

Merging #247 (afcd7e5) into master (558adb8) will increase coverage by 0.05%.
The diff coverage is 97.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
+ Coverage   92.29%   92.34%   +0.05%     
==========================================
  Files          47       49       +2     
  Lines        4038     4078      +40     
==========================================
+ Hits         3727     3766      +39     
- Misses        311      312       +1     
Impacted Files Coverage Δ
src/runtime/context.js 88.33% <90.90%> (-0.18%) ⬇️
src/cql.js 100.00% <100.00%> (ø)
src/elm/expressions.js 100.00% <100.00%> (ø)
src/elm/message.js 100.00% <100.00%> (ø)
src/runtime/executor.js 72.72% <100.00%> (+2.72%) ⬆️
src/runtime/messageListeners.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 558adb8...afcd7e5. Read the comment docs.

@kjmahalingam kjmahalingam merged commit 007629f into master Jun 23, 2021
@kjmahalingam kjmahalingam deleted the message branch June 23, 2021 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants