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

Calendar model #697

Merged
merged 33 commits into from
Oct 30, 2024
Merged

Calendar model #697

merged 33 commits into from
Oct 30, 2024

Conversation

tymmesyde
Copy link
Member

No description provided.

elpiel and others added 20 commits June 11, 2024 17:42
…ems & Extra Prop

Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
…rChanged

Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
Signed-off-by: Lachezar Lechev <lachezar@ambire.com>
@tymmesyde tymmesyde requested a review from elpiel June 24, 2024 12:13
@TRtomasz
Copy link
Member

imo no point of making a pr that does not include all needed dependencies to run. It is untestable. I this case it should include in package.json core-web version that is still in PR without waiting for it to be merged. Preferred solution would be to make this pr only after core and core-web changes are merged so we could avoid testing something without a reason in case it is decided that core/core-web changes should be rewritten or changed in a bigger part.

@elpiel
Copy link
Member

elpiel commented Sep 19, 2024

There are a few things in the PR that I would like to change. Especially with handling the Date changes and date changes with offset. I've worked on some of the changes but left it as I had to work on more pressing issues.

Will get back to this.

}
}

impl From<NaiveDate> for Date {
Copy link
Member

@elpiel elpiel Jun 18, 2024

Choose a reason for hiding this comment

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

This is confusing. NaiveDate also has a day. Why is it different that the DateTime?

  • Please handle Day vs Month values differently

src/models/calendar.rs Show resolved Hide resolved
src/models/calendar.rs Outdated Show resolved Hide resolved
@elpiel
Copy link
Member

elpiel commented Oct 25, 2024

I will still test the stremio-web for the UI implementation and general workings of the logic but there are a few things that need refactoring from the model.
Logic looks good so far though.

src/models/calendar.rs Outdated Show resolved Hide resolved
src/models/calendar.rs Outdated Show resolved Hide resolved
@elpiel elpiel merged commit f063177 into development Oct 30, 2024
3 checks passed
@elpiel elpiel deleted the feat/calendar-model branch October 30, 2024 11:18
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.

3 participants