search

serverless / serverless-local-schedule

⚡️🗺️⏰ Schedule AWS CloudWatch Event based invocations in local time(with DST support!)
MIT License
72 stars 11 forks source link

Fix rate.match is not a function issue #26

Closed KillDozerX2 closed 2 years ago

KillDozerX2 commented 2 years ago

Replaces #21 Fixes #20 Need this for #13

KillDozerX2 commented 2 years ago

@pgrzesik I might have refactored a lot. Let me know if you need some changes.

pgrzesik commented 2 years ago

Hey @KillDozerX2 - thanks a lot for the proposed changes - could we keep it a bit more focused though? I believe all the proposed changes are valuable, but I think it would be great to address one problem per PR as we're usually squashing commits into one to keep the consistent formatting. Would that be okay with you to just include the fix that addresses the rate.match problem and then submit the rest if needed as separate PRs? Thanks in advance and sorry for inconvenience

KillDozerX2 commented 2 years ago

@pgrzesik I've put in a lot and I think I got all refactoring done too. I kinda need the type docs for this and I didn't want to be the guy who pushes a single commit. I don't know if I will be able to do what you're asking. I need to have this working soon along with step functiona support for my org to consider contributing and maintaining.

pgrzesik commented 2 years ago

Hey @KillDozerX2 - I know you've put a lot of work and that is much appreciated 🙇 Sorry for not being clear upfront, but in our projects we usually follow the approach where PRs should ideally be addressing a single issue - here we have a lot of valuable changes, but would you be able to first submit a PR which just fixes the issue with rate.match error and then the other refactorings? Sorry for the inconvenience, but that helps us with maintaining the projects in a bit more structured way

KillDozerX2 commented 2 years ago

@pgrzesik We're moving towards releasing a fork of this separately as we need it to be immediately available for us to start incorporating it in our services. Once our rush settles down, I will start creating PRs here. Sorry for the inconvenience caused.

pgrzesik commented 2 years ago

Thanks a lot @KillDozerX2 - that's totally understandable - we'd be happy to accept contributions whenever you have time 🙇