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

fixes:[issue 1000] Refactor parseRange function in cron file #1091

Merged
merged 9 commits into from
Oct 14, 2024

Conversation

ARYPROGRAMMER
Copy link
Contributor

Description :

This PR introduces improvements to the cron.go file by enhancing the parseRange function to handle more complex cron expressions, including ranges and increments.
The corresponding cron_test.go file has been updated to include comprehensive unit tests that verify the correct behavior of the new parsing logic.
The motivation behind these changes is to ensure that the cron scheduling functionality is robust and can handle a wider variety of cron expressions, thereby increasing its usability for users.

Issue Link

Breaking Changes (if applicable):

There are no breaking changes introduced by this PR. The changes are backward-compatible and extend the existing functionality.
Additional Information:

Additional Information:

The changes leverage the testify package for assertions in the unit tests, ensuring a consistent and readable testing approach.
No additional dependencies or external libraries have been introduced with this change.

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.

pkg/gofr/cron.go Outdated Show resolved Hide resolved
pkg/gofr/cron.go Outdated Show resolved Hide resolved
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
pkg/gofr/cron_test.go Outdated Show resolved Hide resolved
@ARYPROGRAMMER
Copy link
Contributor Author

Please Review the changes and let me know if everything works

@ARYPROGRAMMER
Copy link
Contributor Author

Please see changes for Code Quality and Code Coverage

@ARYPROGRAMMER
Copy link
Contributor Author

@srijan-27 I've re run gofmt -s -w pkg/gofr/cron_test.go please look into the code if code-lint errors occurs again and provide specifications regarding what's been missing

@srijan-27
Copy link
Contributor

@ARYPROGRAMMER - sure, let me check.. Will let you know

@ARYPROGRAMMER
Copy link
Contributor Author

@ARYPROGRAMMER - sure, let me check.. Will let you know

much needed help as even though i have fixed the problem i am running into code quality issues

@srijan-27
Copy link
Contributor

@ARYPROGRAMMER - sure, let me check.. Will let you know

much needed help as even though i have fixed the problem i am running into code quality issues

The checks passed! 🥳

@ARYPROGRAMMER
Copy link
Contributor Author

@ARYPROGRAMMER - sure, let me check.. Will let you know

much needed help as even though i have fixed the problem i am running into code quality issues

The checks passed! 🥳

finally after so many tries, can you get it merged

@srijan-27
Copy link
Contributor

The checks passed! 🥳

finally after so many tries, can you get it merged

The team will validate the changes and merge then.

@ARYPROGRAMMER
Copy link
Contributor Author

@ARYPROGRAMMER - sure, let me check.. Will let you know

much needed help as even though i have fixed the problem i am running into code quality issues

The checks passed! 🥳

ig only your review remains, then it can be merged

@ARYPROGRAMMER
Copy link
Contributor Author

The checks passed! 🥳

finally after so many tries, can you get it merged

The team will validate the changes and merge then.

oh sure thanks a lot appreciate that a lot

Copy link
Contributor Author

@ARYPROGRAMMER ARYPROGRAMMER left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

completed all fixes & tests

@ARYPROGRAMMER
Copy link
Contributor Author

@srijan-27 and @vipul-rawat please review and request to merge

@ARYPROGRAMMER
Copy link
Contributor Author

@Umang01-hash do i need to again update branch or while merging you guys can do it automatically

@Umang01-hash
Copy link
Contributor

@Umang01-hash do i need to again update branch or while merging you guys can do it automatically

If you are planning to push a new commit please do it, if not we will do it while merging

@ARYPROGRAMMER
Copy link
Contributor Author

also @srijan-27 i already made the requested changes so please help me merge this pr as soon as possible

@ARYPROGRAMMER
Copy link
Contributor Author

@Umang01-hash do i need to again update branch or while merging you guys can do it automatically

If you are planning to push a new commit please do it, if not we will do it while merging

thank you so much, i think my current commit is the best one, i look forward to this getting merged soon as its been many days

@Umang01-hash Umang01-hash self-requested a review October 14, 2024 06:16
@Umang01-hash Umang01-hash dismissed srijan-27’s stale review October 14, 2024 06:54

changes have been addressed

@Umang01-hash Umang01-hash merged commit 28699e9 into gofr-dev:development Oct 14, 2024
10 checks passed
@vipul-rawat vipul-rawat linked an issue Oct 28, 2024 that may be closed by this pull request
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.

Refactor parseRange function in cron file
4 participants