-
Notifications
You must be signed in to change notification settings - Fork 106
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
break up kes.override.js - CUMULUS-371-part-1 #237
Conversation
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 like how this is split up now. For the kes overrides, what actual changes did you make to code vs what has just been moved around?
@@ -38,7 +38,7 @@ | |||
"license": "Apache-2.0", | |||
"dependencies": { | |||
"@cumulus/common": "^1.1.0", | |||
"@cumulus/cumulus-message-adapter-js": "0.0.1-beta.3", | |||
"@cumulus/cumulus-message-adapter-js": "^1.0.1", |
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.
+1
@laurenfrederick I just added more tests. The break up was needed so I can use some of the functions in our integrations tests. |
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 read the code, tested locally, and added a few small docs fixes.
Summary: Summary of changes
I'm breaking changes for CUMULUS-371 to smaller PRs. This is the first part.
CUMULUS-371
Changes
Test Plan
Things that should succeed before merging.