Skip to content

downloadMovie endpoint#57

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

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

Conversation

@akash5100
Copy link
Collaborator

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

codecov-commenter commented Aug 14, 2022

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

id: str
format: str
hq: Optional[bool] = False
Copy link
Member

@nabobalis nabobalis Aug 14, 2022

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.

Copy link
Collaborator Author

@akash5100 akash5100 Aug 14, 2022

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
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.

@nabobalis nabobalis merged commit 6c098ea into Helioviewer-Project:main Aug 14, 2022
@nabobalis
Copy link
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