-
Notifications
You must be signed in to change notification settings - Fork 127
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
Adding cloudant change listener #66
Conversation
Signed-off-by: GROEZING <[email protected]>
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.
A general note: Can you please adjust the folder naming and use dashes instead of underscores for the folder?
On the naming of files and folder, I suggest to shrink it to the following set of files:
cloudant-change-listener
|- .ceignore
|- .gitignore
|- build
|- Dockerfile
|- function
| `- function.js
|
|- images
| `- Architecture.png
|
|- job
| |- api-helpers.mjs
| |- job.mjs (formerly cloudant_change_listener.mjs)
| `- package.json
|
|- README.md
`- run
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Review comments considered |
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.
Hi @groezing , thanks for addressing the comments and adjust the structure. I just did a peak review and a small number of places that needs an adjustment due to the re-arrangement of files and folders.
I will do one more round of testing and verify the whole scenario works end to end.
cloudant-change-listener/run
Outdated
echo "Creating the Code Engine daemon job 'cloudant-change-listener-job' ..." | ||
ibmcloud ce job create \ | ||
--name cloudant-change-listener-job \ | ||
--src . \ |
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 option most likely needs to get changed.
--src ./job
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.
The --src is ok, because it must point to the folder where the Dockerfile is located.
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: GROEZING <[email protected]>
Signed-off-by: Enrico Regge <[email protected]>
Signed-off-by: Enrico Regge <[email protected]>
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.
Thanks a ton @groezing for putting so much effort in this sample that provides an end-to-end running solution to demonstrate how to listen on change events in a cloudant database!
LGTM
Adding a new sample as Daemon Job. Also add a reference to the overall Readme.
Overall readme needs to be enhanced with more info about the sample.