Replies: 12 comments
-
|
Copying my answer from #45: I think we should keep both types. These are some of the reasons why I think so:
|
Beta Was this translation helpful? Give feedback.
-
I think it's not a good reason to have an API. Even if so, they may have a bad structure. I mean, we should know the exact structure and reason behind it.
Having exact same logics in different places in the source code is wrong. Imagine an end user sends a VEVENT string with an EXDATE and the developer used rrule, then the result will be absolutely something wrong! Now |
Beta Was this translation helpful? Give feedback.
-
|
This is a good discussion to have and definitely needs to be clarified before going to v1. This is quite a significant change so we should really think through the pros and cons here.
I agree that we should know the exacts reasons for it. Looking at the other
I see your point. But I think just a quick look at the documentation will make the user understand that
This is a good point. Currently the start dates and timezones are defined in the individual recurrence rules it contains (i.e. EXRULES and RRULES), but that is problematic for the |
Beta Was this translation helpful? Give feedback.
-
I think we shouldn't block releasing v1.0 to this discussion. We can release v2.0 one day :)
I think the main reason for the differences between the implementations is that how strict they want to be. Based on the RFC, But I agree with what we have now :) I mean I agree we need to have these 3 structs (maybe with a little different structure). The problem I see is that we shouldn't have On the other hand, we should have
You are right, the main logics are well separated. Do we have a validator for
AFAICS, based on RFC,
And this is what I likeeeee + if we don't have it yet, a validation on RRuleSet. I don't know if we should create a set like PS: One another point regarding my recent changes, after we did the start_dt and tz separation mentioned above, |
Beta Was this translation helpful? Give feedback.
-
|
If we removed start date and timezone fields from Anyways, to summarise I think we both agree on this change:
|
Beta Was this translation helpful? Give feedback.
-
Yes, we can follow this pretty state machine, which I tried once, but exactly because of start date and timezone it was not so pretty :) https://hoverbear.org/blog/rust-state-machine-pattern/
Yes and no.
What are the concerns?
Yes, I agree. If you ask me to have a very short list of the tasks needed to be done (from what I see now)
|
Beta Was this translation helpful? Give feedback.
-
I was thinking that something simpler like this could work as well by just leveraging generics: struct Unvalidated;
struct Validated;
struct RRuleProperties<Stage = Unvalidated> {
freq: Frequency,
...
stage: std::marker::PhantomData<Stage>,
}
impl RRuleProperties<Stage = Unvalidated> {
fn validate(self) -> Result<Self<Validated>, ValidatonError> {
....
}
}
struct RRule {
dt_start: Datetime,
properties: RRuleProperties<Validated>
}Then we do not have to define so boilerplate "machinery" code, but we can ofc discuss impl details further when we have created the issue/PR.
My concern is that I feel that it would make the API more awkward if we have to call
Yes!
Let's specify what exactly needs to be validated for an
Not yet, more thinking / discussions needed.
Not yet, more thinking / discussions needed.
Not exactly sure what you mean here, but seems like something that can be described in a new issue when we get to a stage where we can implement this. |
Beta Was this translation helpful? Give feedback.
-
Your code is almost that pretty state machine, and I agree with the code you wrote.
This
So a
I meant like the code sample you put here. So seems like we already kinda agreed on this :D |
Beta Was this translation helpful? Give feedback.
-
|
I am experimenting a little bit with how the public API would look like if we removed start date from struct RRuleProperties<S> {
freq: Frequency,
...
}
impl RRuleProperties<Validated> {
pub(crate) fn into_iter(self, dt_start: Datetime, tz: Tz) -> RRuleIter {}
}
struct RRuleSet {
pub rrule: Vec<RRuleProperties<Validated>>,
pub rdate: Vec<DateTime>,
pub exrule: Vec<RRuleProperties<Validated>>,
pub exdate: Vec<DateTime>,
pub dt_start: DateTime,
pub tz: Tz,
}
// Usage 1: Construction from properties
let mut set = RRuleSet::new(dtstart);
let props = RRuleProperties {
freq: Daily,
..Default::default()
};
set.rrule(props).unwrap(); // RRuleSet::rule returns Result<(), ValidationError>
set.all(100);
// Usage 2: Construction from string
let mut set: RRuleSet = "DTSTART:...".parse().unwrap(); // If parsing from string was successful, then we know the RRuleSet is valid.
set.all(100);
// Usage 3: Serde
#[derive(Deserialize, Serialize)]
struct RequestBody {
rrule_set: RRuleSet // If deserializing was successful, then we know the RRuleSet is valid.
} |
Beta Was this translation helpful? Give feedback.
-
|
I agree generally. I'm also experiencing some things. It's a huge change, everywhere :D I think I also agree that |
Beta Was this translation helpful? Give feedback.
-
Agreed. @ralpha do you have any immediate concerns with not exposing I think my only concerns are these:
|
Beta Was this translation helpful? Give feedback.
-
We still can make I'm saying, since we have it, let's make it public for somebody who required that (including myself, maybe :D).
I think I disagree to have an option to disable them. Mostly people will generate |
Beta Was this translation helpful? Give feedback.
-
Hello,
What are the differences between
RRuleSetandRRule?Can we merge them? (or always use
RRuleSetif it's a superset forRRule)What are the equivalent RFC standard of them? (this?)
Beta Was this translation helpful? Give feedback.
All reactions