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

Video info scanner refactoring #5332

Closed
wants to merge 11 commits into from

Conversation

tamland
Copy link
Member

@tamland tamland commented Sep 7, 2014

This is an attempt at untangling this spaghetti. Scanning movie folders vs tv folders hardly have anything in common at this point, making it impossible to follow. Please see individual commits for step-by-step.

If I have done this correctly, there should be no bug fixes or behavior changes in these commits.

@mkortstiege
Copy link
Member

Finally. Looks fine from a brief look. I´ll check in detail, run some tests and report back.

@Montellese
Copy link
Member

Any refactor in this area is very much welcome. I'm not very knowledgeable in this code though so I don't feel comfortable commenting on the "unreachable" code that has been removed. Some of it simply becomes unreachable due to the changes made in this PR and I can't judge if these changes are alright or not.

@tamland
Copy link
Member Author

tamland commented Sep 8, 2014

@Montellese I tried to do this very incremental so it would be possible to follow. The first two commits just copy-paste DoScan, then the tv part of movie scan, and vice versa, is removed. The 'unreachable' part is because of the content checks.

@MartijnKaijser MartijnKaijser added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Fix non-breaking change which fixes an issue v15 Isengard labels Nov 12, 2014
@MartijnKaijser
Copy link
Member

@tamland @mkortstiege would be nice if we could get this cleanup soon so we can have wide testing for regressions.

@tamland
Copy link
Member Author

tamland commented Dec 25, 2014

Too much conflict. Will rather redo if we don't come up with something better;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Fix non-breaking change which fixes an issue v15 Isengard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants