Skip to content
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

Added scheduled event #299

Closed
wants to merge 6 commits into from
Closed

Added scheduled event #299

wants to merge 6 commits into from

Conversation

exocom
Copy link

@exocom exocom commented Jul 26, 2018

Issue , if available:
Closes #16. You will have to bring your own class based on the data returned from your given trigger.

Description of changes:
Added missing class for Scheduled Events.
https://docs.aws.amazon.com/lambda/latest/dg/eventsources.html#eventsources-scheduled-event

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@exocom exocom mentioned this pull request Jul 28, 2018
Kalarrs Topham and others added 3 commits July 28, 2018 12:26
…implment interface. Also created the ECS Event. Leaving 25 other events up to others to contribute.
@normj
Copy link
Member

normj commented Aug 4, 2018

Sorry I haven't responded sooner. Trying to clear a few other things off my plate then I'll take a deeper look. I do appreciate the PR.

@normj
Copy link
Member

normj commented Sep 14, 2018

Is there a reason not to use a base class instead only having an interface? With only the interface we have to copy everyone of the those top level properties for each event type which there are a lot.

I'm also wondering if we could just have one CloudWatch Logs project which has the interface, base class and a collection of implementations like the ECS classes you have. That would make it much easier to add more types and not have to do all of the boiler plate work of creating a project. The classes are so small I can't imagine they are going to add any significant impact to the size.

@exocom
Copy link
Author

exocom commented Sep 19, 2018

@normj Going to do some testing on interface vs abstract class. Once I've completed I'll let you know what we find.

As far as single project, that sounds great. It would be a lot easier for publishing to Nuget.

I'm going to close this PR and reopen a new PR with the discussed changes. It will take a couple of days.

@exocom exocom closed this Sep 19, 2018
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.

3 participants