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

Added ImdbId Endpoints on MovieService & Overloads to make language optional #81

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ProIcons
Copy link
Contributor

  • Added ImdbId to Endpoints so you can query a movie by it's imdb id.
  • Removed Language query parameter on Movie ExternalIds endpoint as is not a valid query option for this endpoint.
  • Added more Overloads to make Language query truly optional

…ptional.

- Added ImdbId to Endpoints so you can query a movie by it's imdb id.
- Removed Language query parameter on Movie ExternalIds endpoint as is not a valid query option for this endpoint.
- Added more Overloads to make Language query truly optional
@UweTrottmann
Copy link
Owner

First of all thanks. Some remarks:

  • The API docs mention nothing about supporting IMDB IDs https://developers.themoviedb.org/3/movies/get-movie-details Where does the info come from that this is supported?
  • I'd rather not add too many new overloads. This is just a maintenance burden down the road. It's not that hard to e.g. just pass null for the language parameter.

@ProIcons
Copy link
Contributor Author

ProIcons commented Aug 27, 2020

I've read this on the forum , it's undocumented You can try it thought it works. About the language as you see i didn't changed it everywhere, i just worked on the MovieService, and on the first endpoints of the PersonService. Which are the most important Services
image

@UweTrottmann
Copy link
Owner

I'd rather not support this. The docs also say to use find to look up external IDs. https://developers.themoviedb.org/3/getting-started/external-ids

i just worked on the MovieService, and on the first endpoints of the PersonService

How does this address my point about maintenance burden? If it's that much of an issue, I'd suggest switching to a builder pattern.

@ProIcons
Copy link
Contributor Author

It's up to you. Whatsoever it's working just fine and removing the need of an extraa query and potentially more Latency along the way on the user's app.

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.

2 participants