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.

Sign up for GitHub

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

Jump to bottom

Add support for cron-tz #1369

Open
wants to merge 17 commits into
base: development
Choose a base branch
from
Open

Add support for cron-tz #1369

wants to merge 17 commits into from

Conversation

JayGoyal96
Copy link

Pull Request Template

Description:

  • Provides a solve for issue Support for CRON_TZ #1362
  • Updated NewCron factory to support functional options and added a functional option to specify time.Location for the crontab.
  • This ensures the backward compatibility.

Checklist:

  • I have formatted my code using goimport and golangci-lint.
  • All new code is covered by unit tests.
  • This PR does not decrease the overall code coverage.
  • I have reviewed the code comments and documentation for clarity.

@Umang01-hash
Copy link
Contributor

@JayGoyal96 Can you please also update it in the documentation so that other people get aware about it.

ccoVeille
ccoVeille reviewed Jan 12, 2025
View reviewed changes
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
ccoVeille
ccoVeille reviewed Jan 13, 2025
View reviewed changes
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
@Umang01-hash
Copy link
Contributor

Screenshot 2025-01-13 at 2 33 35 PM @JayGoyal96 the pipeline is failing due to this linter error, please resolve.

@ccoVeille
Copy link
Contributor

Please fix the PR title

support instead of supoort

Depending on if the PR will be squashed or not, the message of the first commit will also have to be fixed

@JayGoyal96 JayGoyal96 changed the title Add supoort for cron-tz Add support for cron-tz Jan 14, 2025
coolwednesday
coolwednesday suggested changes Jan 15, 2025
View reviewed changes
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
@coolwednesday
Copy link
Contributor

@JayGoyal96 , please fix the code quality issue and we can go ahead and get the PR merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coolwednesday coolwednesday coolwednesday approved these changes

@Umang01-hash Umang01-hash Umang01-hash approved these changes

@aryanmehrotra aryanmehrotra aryanmehrotra approved these changes

@ccoVeille ccoVeille ccoVeille approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JayGoyal96 @Umang01-hash @ccoVeille @coolwednesday @aryanmehrotra