swift-server / swift-aws-lambda-runtime

Swift implementation of AWS Lambda Runtime

Apache License 2.0

1.13k stars 102 forks source link

Updated S3.Event #195

Closed mufumade closed 3 years ago

mufumade commented 3 years ago

Motivation:

194

Modifications:

Changed the S3.Event object size field to an optional due to the missing field on a s3:ObjectRemoved:* event Added one additional test to check propper json decoding

Result:

s3 event json will now decode successfully even if aws does not populate the size field.

swift-server-bot commented 3 years ago

Can one of the admins verify this patch?

swift-server-bot commented 3 years ago

Can one of the admins verify this patch?

swift-server-bot commented 3 years ago

Can one of the admins verify this patch?

swift-server-bot commented 3 years ago

Can one of the admins verify this patch?

swift-server-bot commented 3 years ago

Can one of the admins verify this patch?

swift-server-bot commented 3 years ago

Can one of the admins verify this patch?

mufumade commented 3 years ago

@fabianfett Thanks for the feedback!

fabianfett commented 3 years ago

@swift-server-bot test this please

fabianfett commented 3 years ago

@mufumade Would you mind running swiftformat . once in the project folder, to please the soundness check?

mufumade commented 3 years ago

So this was a bit strange. At first swiftformat changed like 30 files although I only changed 2. On the second attempt swiftformat changed only the comment you provided with your suggestions. I hope it's all fine now.

fabianfett commented 3 years ago

@mufumade oh. sorry. it looks to me like our swiftformat is a little out of date. I'm updating it right now and will post another pr. Once that swiftformat pr has landed, we update this pr and run swiftformat again, which then should only affect your changed files. Sorry for the hassle and confusion.

fabianfett commented 3 years ago

For reference: https://github.com/swift-server/swift-aws-lambda-runtime/pull/196

fabianfett commented 3 years ago

@mufumade Would you mind resolving the merge conflicts and running swiftformat again? After that we should be ready to merge.

tomerd commented 3 years ago

@swift-server-bot test this please

mufumade commented 3 years ago

@fabianfett have a look at the newest commit.

fabianfett commented 3 years ago

@swift-server-bot test this please

fabianfett commented 3 years ago

@mufumade 👋 Ping: Are you still interested in getting this merged? I would love to see this land...

mufumade commented 3 years ago

Oh. Sorry for the delay. I'll get started right away.

mufumade commented 3 years ago

@fabianfett So i merged the current main into my branch, ran swiftformat again to check but I am not quite sure what you mean by re-adding the dropped internal acls?

fabianfett commented 3 years ago

@swift-server-bot test this please

fabianfett commented 3 years ago

@swift-server-bot test this please