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

Refactor: Apply extract ids for search results #10

Closed
wants to merge 4 commits into from
Closed

Conversation

glensc
Copy link
Owner

@glensc glensc commented Nov 30, 2022

Continue #9

@glensc glensc self-assigned this Nov 30, 2022
@glensc glensc changed the title Extract ids for show search results Refactor: Apply extract ids for search results Nov 30, 2022
@glensc glensc marked this pull request as ready for review November 30, 2022 21:59
@glensc glensc requested a review from simonc56 November 30, 2022 21:59
@glensc
Copy link
Owner Author

glensc commented Nov 30, 2022

I don't really understand what the extract_ids do, as code before and after continues to work. but I would like the code to be consistent.

next step is to use MixIn to simplify things:

trakt/sync.py Outdated Show resolved Hide resolved
self._items.append(Person(item_data['name'],
item_data['slug']))
extract_ids(item_data)
self._items.append(Person(**item_data))

yield self._items
Copy link
Owner Author

Choose a reason for hiding this comment

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

I noticed that UserList.get_items() has a bug, it always appends items to self._items. IMHO it should reset self._items = [] on each call to get_items().

@glensc glensc requested a review from simonc56 December 11, 2022 14:26
@glensc
Copy link
Owner Author

glensc commented Dec 11, 2022

Probably useless after #14

@glensc glensc closed this Dec 11, 2022
@glensc glensc deleted the pts-1244-fix-v4 branch December 11, 2022 15:25
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