Skip to content

Added class to set S3 events#393

Merged
DeviaVir merged 4 commits intomotdotla:masterfrom
abetomo:feature_s3_events
Dec 7, 2017
Merged

Added class to set S3 events#393
DeviaVir merged 4 commits intomotdotla:masterfrom
abetomo:feature_s3_events

Conversation

@abetomo
Copy link
Copy Markdown
Contributor

@abetomo abetomo commented Dec 7, 2017

Added class to set s3 events.
In the next PR add S3Events to main.js.

Comment thread package.json
"test": "standard && standard bin/node-lambda && mocha"
"lint": "standard && standard bin/node-lambda",
"test": "npm run lint && npm run unit",
"unit": "mocha"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It has nothing to do with subject matter, but cleaned the scripts.

Copy link
Copy Markdown
Collaborator

@DeviaVir DeviaVir left a comment

Choose a reason for hiding this comment

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

Just some language feedback, looks good to me 👍

Comment thread lib/s3_events.js Outdated
'use strict'

/**
* Do not create Bucket of S3
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do not create S3 bucket?

Comment thread lib/s3_events.js Outdated
if (err) {
if (err.code !== 'ResourceConflictException') reject(err)
// If it exists it will result in an error but there is no problem.
resolve('Already exists permission')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Permission already set

@abetomo
Copy link
Copy Markdown
Contributor Author

abetomo commented Dec 7, 2017

@DeviaVir Thank you!
Your advice was very helpful.

@DeviaVir DeviaVir merged commit 286e063 into motdotla:master Dec 7, 2017
@abetomo abetomo deleted the feature_s3_events branch December 7, 2017 14:33
@DeviaVir DeviaVir mentioned this pull request Dec 11, 2017
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.

2 participants