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.

Sign up for GitHub

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

Jump to bottom

Improve scene_exceptions #848

Merged
merged 15 commits into from
Sep 10, 2016
Merged

Improve scene_exceptions #848

merged 15 commits into from
Sep 10, 2016

Conversation

medariox
Copy link
Contributor

@medariox medariox commented Aug 4, 2016

  • PR is based on the DEVELOP branch
  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
  • Read contribution guide

@medariox medariox added this to the 0.1.4 milestone Aug 4, 2016
@fernandog
Copy link
Contributor

@medariox still in progress?

@medariox
Copy link
Contributor Author

medariox commented Aug 9, 2016

@fernandog
Yeah, still working on it. I fixed most issues, but need some more time to refactor the code.

@fernandog
Copy link
Contributor

@medariox milestone 0.1.5 then?

@medariox medariox mentioned this pull request Aug 17, 2016
Closed
@fernandog fernandog modified the milestones: 0.1.4, 0.1.5 Aug 21, 2016
@medariox
Copy link
Contributor Author

medariox commented Sep 1, 2016

This PR didn't make it as far as I'd have wished. Don't really have much time to clean up the rest of the code and since it should solve this bug as well, I'll leave it for now.

@fernandog Can you please rebase?

@medariox medariox added Concluded Bug Needs review Needs testing Requires testing to make sure it's working as intended and removed In progress labels Sep 1, 2016
@fernandog fernandog force-pushed the improve-scene-ex branch 2 times, most recently from b2275cf to 9aebbb0 Compare September 5, 2016 15:48
@fernandog
Copy link
Contributor

no one else is going to test this. imo it should be merged to develop so everybody test it

@fernandog fernandog self-assigned this Sep 8, 2016
@fernandog
Copy link
Contributor

fernandog commented Sep 10, 2016 •
edited
Loading

@medariox why not always erase scene_exceptions before adding new ones (if succeed to fetch from github)? What do you think?

so if a scene name exception becomes invalid, when we remove from file the DB won't have the wrong/old one

fernandog
fernandog reviewed Sep 10, 2016
View reviewed changes
sickbeard/scene_exceptions.py
def update_scene_exceptions(indexer_id, scene_exceptions, season=-1):
"""Given a indexer_id, and a list of all show scene exceptions, update the db."""
cache_db_con = db.DBConnection('cache.db')
cache_db_con.action(b'DELETE FROM scene_exceptions WHERE indexer_id=? and season=?', [indexer_id, season])
Copy link
Contributor

@fernandog fernandog Sep 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@medariox seems we already does it here.

so don't understand why we had duplicates

UPDATE: maybe we can remove the and season=?'

@medariox
Copy link
Contributor Author

@fernandog
I'm not sure about this. Is this safe?

@fernandog
Copy link
Contributor

yes, it is

@fernandog
Copy link
Contributor

fernandog commented Sep 10, 2016 •
edited by duramato
Loading

Approved

Approved with PullApprove

@fernandog fernandog removed the Needs testing Requires testing to make sure it's working as intended label Sep 10, 2016
@fernandog fernandog merged commit b6657eb into develop Sep 10, 2016
@fernandog fernandog deleted the improve-scene-ex branch September 10, 2016 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@fernandog fernandog

Labels
Bug Concluded Enhancement
Projects
None yet
Milestone
0.1.5
Development

Successfully merging this pull request may close these issues.
2 participants
@medariox @fernandog