Skip to content
This repository has been archived by the owner on Apr 21, 2022. It is now read-only.

Feature/parse rrule to json #2

Merged
merged 8 commits into from
Mar 14, 2019
Merged

Conversation

shiveenp
Copy link
Contributor

Description

Enable parsing of RRule to Json

@ordermentum
Copy link

ordermentum bot commented Mar 14, 2019

Please make sure to add screenshots or gifs of the change to give a clear
picture of the change. If a design PR was made please make sure to link to it along with the
JIRA ticket. A Design PR will avoid unnecessary rework that might come up in a PR.

Good Pull Requests adhere to a single responsibility.

  • Break big changes into smaller Pull Requests
  • Big changes increase risk - smaller PRs are easier to review
  • Refactoring changes should be in seperate PRs to avoid noise
  • A story/ticket can have more than 1 PR

Some guidelines on giving and receiving feedback in Code Reviews

.expect("unsuccessful parse") // unwrap the parse result
.next().unwrap();
.next()
.unwrap();

Choose a reason for hiding this comment

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

.map_err is your friend with https://boats.gitlab.io/failure/ to handle parsing errors.

@shiveenp shiveenp marked this pull request as ready for review March 14, 2019 03:11
rrule_string: "FREQ=DAILY",
expected_flat_json: r#"{"frequency":"DAILY"}"#,
},
RRuleTestCase {

Choose a reason for hiding this comment

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

you could write this as a macro. but that is for another day... 💃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! will try to incorporate in the next PR

@shiveenp shiveenp merged commit d2e2d13 into master Mar 14, 2019
@shiveenp shiveenp deleted the feature/parse-rrule-to-json branch March 14, 2019 03:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants