Skip to content

feat: adding holidays to calendar as events #165

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

Merged
merged 34 commits into from
Feb 11, 2021

Conversation

galzunited
Copy link
Contributor

https://forums.pythonic.guru/t/topic/8637

Added an option for user to import holidays from ics file.
Took the naïve approach for now - not checking for duplicates while importing.
Need to think about this mechanism - since checking every date to see if holiday is there is too expensive.
As for the first step i think its ok (:

Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

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

Great job! I've added few of my insights :)

Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

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

Great job!

@galzunited galzunited requested a review from yammesicka February 4, 2021 16:32
Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

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

Suggested only a minor fix. Please add type annotations to your functions and we're good to go :)

@galzunited galzunited requested a review from yammesicka February 9, 2021 10:46
Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

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

Awesome job! I've added few additional comments :)

@codecov-io
Copy link

codecov-io commented Feb 10, 2021

Codecov Report

Merging #165 (b712b4b) into develop (dfd34f3) will decrease coverage by 0.35%.
The diff coverage is 82.60%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #165      +/-   ##
===========================================
- Coverage    99.38%   99.02%   -0.36%     
===========================================
  Files           46       46              
  Lines         2101     2146      +45     
===========================================
+ Hits          2088     2125      +37     
- Misses          13       21       +8     
Impacted Files Coverage Δ
app/routers/profile.py 93.84% <82.60%> (-6.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfd34f3...b712b4b. Read the comment docs.

@yammesicka yammesicka merged commit 856c843 into PythonFreeCourse:develop Feb 11, 2021
@galzunited
Copy link
Contributor Author

galzunited commented Feb 13, 2021

Hi @yammesicka - just found out that in latest version I'm having weird issues - when trying to add the file and import I'm getting:
"Did not find CR at end of boundary (40)"
While trying to import using postman everything works fine (and it also worked locally when originally pushed).
Tried to look for answers without success, might relate to multipart, but in django it seems we are using to correct enctype for the form.

Edit - found the solution - i pushed it to extending_openapi as this branch already been marged

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.

4 participants