search

Kralizek / AWSLambdaSharpTemplate

A template for AWS Lambda functions written in C# with .NET Core
MIT License
61 stars 25 forks source link

Improve customization of service registration #17

Closed Thomas-X closed 1 year ago

Thomas-X commented 3 years ago

The idea for this PR is so you can pass your own serialization logic incase your incoming messages are in an other format than JSON.

I added unit tests too, but let me know if there's any questions or remarks on how it's structured. README is updated to reflect the (optional) API changes as well

Kralizek commented 3 years ago

Hi @Thomas-X Thank you for your pull request. I will be checking the PR thoroughly sometimes this week (hopefully already tomorrow but I can't promise).

In the meantime, I would appreciate if you could revert some codestyle changes that slipped through when you committed your changes.

I'm asking this because it makes it easier to understand what was changed in the PR.

Thomas-X commented 3 years ago

Changed some of the formatting that slipped through, maybe it could be worth adding a .editorconfig for the future

Kralizek commented 3 years ago

Hi @Thomas-X, sorry for the delay.

I have been thinking a bit about this PR and I think that adding (yet) another parameter to the registration methods is not sustainable in the long term.

I'm considering the idea of using a configuration delegate that can be used to customize the registered services as needed.

Something like below

services.UseNotificationHandler(c => c.UseSerializer<MyCustomSerializer>().EnableParallelExecution());

So my plan is to rewrite this PR with this design but I am not sure when I have time to do so.

Thomas-X commented 3 years ago

Hi @Kralizek, no problem, we all have lives :).

I think that using a configuration delegate definitely makes it more future-proof and more extensible, so we should rewrite this PR for that.

I'm a bit dry on time as well, but I can take a look at the idea next week(end) probably, and see if I'm able to implement it! (I'm a junior dev). Unless you beat me to the punch, of course

Kralizek commented 3 years ago

@Thomas-X managed to squeeze a nick of time for the lib part. tests need to be updated as the build is failing now.

Thomas-X commented 3 years ago

@Kralizek That's really awesome, I'll see what I can do soon to get it built and the tests green

Kralizek commented 3 years ago

Could you add an implementation of ISerializer for JSON so that we skip that ugly if?

Kralizek commented 3 years ago

Left to do before merge:

Left to do before new release:

Kralizek commented 1 year ago

Hey @Thomas-X I was working on this lib and by mistake I fucked up your PR while rebasing it after I merged in #22

I am not able to reopen this PR. I have the code locally but I'd love if you would open a new PR so that you can take credit on the work you did.

Best regards and sorry =)