search

aspnet / WebHooks

[Archived] Libraries to create and consume web hooks on ASP.NET Core. Project moved to https://github.com/aspnet/AspLabs
Apache License 2.0
627 stars 439 forks source link

Migrate WebHooks sender package to ASP.NET Core #301

Closed Havret closed 6 years ago

Havret commented 6 years ago

183

Hi,

I've done the initial work to migrate sender package to ASP.NET Core. I have been battle-testing this for several months and it seems to work pretty nice. But I have noticed one serious drawback in the current design. It is the decision to send webhook payload data as two lists: one containing keys and one containing values. It really drives me crazy when I have to reintegrate the payload on the client. Why don't we just post JSON serialized data? I would get rid of NotificationDictionary entirely and let a user send any data he/she wants.

Please tell if my work will be any use for this project, or if you guys want to redesign this functionality from scratch as you did with receivers part.

For some reason when I added the files to WebHooks solution I would not have been able to make InternalsVisibleTo works. As a result, the test project doesn't compile hence it uses some internal from Sender Package. :( I need some help with this.

dnfclas commented 6 years ago

CLA assistant check
All CLA requirements met.

ArturWincenciak commented 6 years ago

Yes, definitely Notification Dictionary should be deleted.

Havret commented 6 years ago

@dougbu Could you take a look at it?

dougbu commented 6 years ago

@Havret thank-you very much for the effort you've put into this proposal. On our side, reviewing this PR and investigating alternative designs would require a significant investment. We are not planning to make that investment at this time.

Please feel free to make this work available in another fashion.

Havret commented 6 years ago

Thanks for the reply. I will continue the work on my fork. then.