Skip to content

Conversation

@KanniShashankh
Copy link
Contributor

@KanniShashankh KanniShashankh commented Sep 14, 2023

What does this PR do?

Updated the Regex Pattern to correctly validate the cron syntax.
Mentioned the allowed values in a comment.

Test Plan

I tried a few statements and they validated correctly.

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yes

@vercel
Copy link

vercel bot commented Sep 14, 2023

@KanniShashankh is attempting to deploy a commit to the appwrite Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@navneetbz navneetbz left a comment

Choose a reason for hiding this comment

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

LGTM

@vercel
Copy link

vercel bot commented Sep 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
console ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 4:33pm
console-cloud ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 4:33pm
console-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 4:33pm
console-preview-cloud ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 4:33pm

Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

Great work! Please make sure the linter and tests pass.

@KanniShashankh
Copy link
Contributor Author

Great work! Please make sure the linter and tests pass.

Done with the linter test, Investigating the problem in Unit testing which incorrectly validates the tests. Will push a commit soon

@stnguyen90
Copy link
Contributor

@KanniShashankh there still seems to be a problem with the linter 🧐

@KanniShashankh KanniShashankh force-pushed the bug-5719-client-cron-regex-update branch from 5f7eda3 to c3faa36 Compare October 11, 2023 13:05
@KanniShashankh
Copy link
Contributor Author

@stnguyen90 To keep the regex simple, since this is just a frontend check, i have very much simplified the expression to match 5 segments of cron. Please let me know if i need to add something :)

@TorstenDittmann TorstenDittmann dismissed stnguyen90’s stale review October 18, 2023 11:01

seems to be resolved

@TorstenDittmann TorstenDittmann merged commit 07da7e2 into appwrite:main Oct 18, 2023
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.

🐛 Bug Report: Schedule function with minutes does not work

4 participants