Skip to content

downloadMovie endpoint#57

Merged
nabobalis merged 4 commits into
Helioviewer-Project:mainfrom
akash5100:movies_endpoint
Aug 14, 2022
Merged

downloadMovie endpoint#57
nabobalis merged 4 commits into
Helioviewer-Project:mainfrom
akash5100:movies_endpoint

Conversation

@akash5100

Copy link
Copy Markdown
Collaborator

@akash5100 akash5100 self-assigned this Aug 14, 2022
@codecov-commenter

codecov-commenter commented Aug 14, 2022

Copy link
Copy Markdown

Codecov Report

Merging #57 (085b409) into main (c99b633) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #57      +/-   ##
==========================================
+ Coverage   99.04%   99.07%   +0.03%     
==========================================
  Files          18       19       +1     
  Lines         314      326      +12     
==========================================
+ Hits          311      323      +12     
  Misses          3        3              
Impacted Files Coverage Δ
hvpy/api_groups/movies/download_movie.py 100.00% <100.00%> (ø)
hvpy/facade.py 96.55% <100.00%> (+0.25%) ⬆️
hvpy/parameters.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@akash5100
akash5100 requested a review from nabobalis August 14, 2022 04:49
Comment thread hvpy/api_groups/movies/download_movie.py Outdated
Comment thread hvpy/api_groups/movies/download_movie.py Outdated

id: str
format: str
hq: Optional[bool] = False

@nabobalis nabobalis Aug 14, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
hq: Optional[bool] = False
hq: bool = False

Optional is only if the value can be set to None, it does not mean if its optional from a user's point of view.

@akash5100 akash5100 Aug 14, 2022

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Okay, understood
Because in the facade function we already have the Optional parameter, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No. You set the default value, that makes it optional.

Comment thread hvpy/parameters.py Outdated
Comment thread hvpy/api_groups/movies/download_movie.py Outdated
Comment thread hvpy/api_groups/movies/download_movie.py Outdated
Comment thread hvpy/facade.py Outdated
@nabobalis
nabobalis merged commit 6c098ea into Helioviewer-Project:main Aug 14, 2022
@nabobalis

Copy link
Copy Markdown
Member

Another one down!

@akash5100
akash5100 deleted the movies_endpoint branch August 14, 2022 06:15
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.

Movies - downloadMovie endpoint

3 participants