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

Fix season search in tvcuk #497

Merged
merged 1 commit into from
May 7, 2016
Merged

Fix season search in tvcuk #497

merged 1 commit into from
May 7, 2016

Conversation

duramato
Copy link
Contributor

@duramato duramato commented Apr 27, 2016

No description provided.

@@ -102,7 +100,7 @@ def search(self, search_strings, age=0, ep_obj=None): # pylint: disable=too-man
for search_string in search_strings[mode]:

if mode == 'Season':
search_string = re.sub(ur'(.*)Season', ur'\1Series', search_string)
search_string = re.sub(ur'(.*)S0?', ur'\1Series', search_string)
Copy link
Contributor

@fernandog fernandog Apr 27, 2016

Choose a reason for hiding this comment

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

@duramato can you add comments with examples of releases that matches it ? Also s10 wont match, right?

what about releases like Show1.Season.6...... ?

Copy link
Contributor Author

@duramato duramato Apr 27, 2016

Choose a reason for hiding this comment

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

There are no Season 6, just Series 6.
They are all like that or at least 90%, making the 10% despicable
It always replaced them but replaced wrong

Copy link
Contributor

@labrys labrys Apr 30, 2016

Choose a reason for hiding this comment

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

What about seasons > 9? And series with a year for a season?
How about ur'\bs\d{1,4}\b' instead? It will match any Sx through Sxxxx. Also since it doesn't have the .* it will match multi-season too.

Copy link
Contributor Author

@duramato duramato Apr 30, 2016

Choose a reason for hiding this comment

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

Would it bring any advantage? The search string has multiple seasons?
With the current one it will only replace S to Series and if its a season shorter than 9 S0 to Series becouse in the site all seasons less than 10 dont have a 0 and if its searched with it no results will be returned
Also there arent any seasons with year from my research in the site

@labrys
Copy link
Contributor

labrys commented May 6, 2016

@duramato If this tested good i think its ready for rebase and merge.

@labrys labrys added the Needs testing Requires testing to make sure it's working as intended label May 6, 2016
Remove useless code
@fernandog fernandog removed the Needs testing Requires testing to make sure it's working as intended label May 7, 2016
@fernandog
Copy link
Contributor

fernandog commented May 7, 2016

Approved

Approved with PullApprove

@labrys labrys closed this May 7, 2016
@labrys labrys reopened this May 7, 2016
@labrys
Copy link
Contributor

labrys commented May 7, 2016

Approved

Approved with PullApprove

@labrys
Copy link
Contributor

labrys commented May 7, 2016

Attempted to kickstart QC by closing and re-opening with no luck. Forcing merge since QC is failing to report status.

@labrys labrys merged commit dbd3c22 into develop May 7, 2016
@labrys labrys deleted the tvcuk branch May 7, 2016 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants