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

feat: add market_schedule module #112

Merged
merged 27 commits into from
Apr 12, 2024
Merged

feat: add market_schedule module #112

merged 27 commits into from
Apr 12, 2024

Conversation

keyvankhademi
Copy link
Contributor

@keyvankhademi keyvankhademi commented Apr 8, 2024

This add marker_schedule module and renames the old schedule to legacy_schedule.
It uses a new metadata key schedule and it is backward compatible by falling back to weekly_schedule metadata key.

@keyvankhademi keyvankhademi self-assigned this Apr 8, 2024
Copy link
Contributor

@Reisen Reisen left a comment

Choose a reason for hiding this comment

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

I might be missing context, but is the string format for these holiday schedules defined anywhere or is it being introduced by this PR? I think it would be worth using a parsing library here to make the parsing a bit more robust. While FromStr is the right place to put this code I think handling parsing more robustly via something like winnow might be the right thing to do to define this format more concretely.

src/agent/holiday_hours.rs Outdated Show resolved Hide resolved
src/agent/holiday_hours.rs Outdated Show resolved Hide resolved
src/agent/holiday_hours.rs Outdated Show resolved Hide resolved
src/agent/holiday_hours.rs Outdated Show resolved Hide resolved
src/agent.rs Outdated Show resolved Hide resolved
src/agent.rs Outdated Show resolved Hide resolved
src/agent/schedule.rs Outdated Show resolved Hide resolved
src/agent/schedule.rs Outdated Show resolved Hide resolved
@keyvankhademi keyvankhademi changed the title feat: add holiday_hours module feat: add market_schedule module Apr 11, 2024
Copy link
Collaborator

@ali-bahjati ali-bahjati left a comment

Choose a reason for hiding this comment

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

Very nice!

src/agent/legacy_schedule.rs Outdated Show resolved Hide resolved
@keyvankhademi keyvankhademi merged commit 920df19 into main Apr 12, 2024
3 checks passed
@keyvankhademi keyvankhademi deleted the holiday_hours branch April 12, 2024 17:55
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.

5 participants