-
Notifications
You must be signed in to change notification settings - Fork 274
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
Ajout d'une option de blacklist de résolution pour les films #4026
base: Beta
Are you sure you want to change the base?
Conversation
@@ -34,7 +34,7 @@ | |||
UNCLASSIFIED = 'Indéterminé' | |||
|
|||
MOVIE_MOVIE = (URL_MAIN + '&sMedia=film', 'showMenuFilms') | |||
# MOVIE_NEWS = (URL_MAIN + '&sMedia=film&sYear=2022', 'showMovies') | |||
#MOVIE_NEWS = (URL_MAIN + '&sMedia=film&sYear=2022', 'showMovies') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pourquoi avoir supprimé l'espace ici ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surement une mauvaise manip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Du coup, dans cette PR, la blacklist n'est appliquée que a pastbin si je comprend bien 🤔
for res in listRes: | ||
# Set bValid = True if sRest = BLACKLIST APPLIED because it is like we got each sRes in res | ||
if sRes == "BLACKLIST APPLIED": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi deux conditions ?
for res in listRes:
if sRes == "BLACKLIST APPLIED" or sRes in res:
bValid = True
break
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ça remonte à loin, je suppose que la condition normale était déjà là et je ne voulais pas interférer avec le code existant, mais en effet, ça devrait être mis en une seule condition sans problème.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je pense que ce sera plus propre de définir une constante avec la valeur:
BLACKLIST APPLIED
. Du genre:
BLACKLIST_APPLIED = "BLACKLIST APPLIED"
# Et donc dans le code:
sUrl = siteUrl + '&sRes=' + BLACKLIST_APPLIED
# ...
if sRes == BLACKLIST_APPLIED:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ce serait plus cohérent avec le reste du code, je crois avoir vu que le sRes est également une variable.
Hello @detobel36 Je note les modifs, mais actuellement, j'ai peu de temps pour les faire. Si tu veux les faire de ton côté, pas de souci, il n'y a pas de problème. |
Hello, A priori, je n'ai pas les droits de push sur ta branche (a moins que tu me donnes accès à ton repos) xD Moi personnellement, ça ne me dérange pas si ça prend du temps. Ce sont juste des remarques/conseils (d'ailleurs désolé si j'ai été un peu "direct" (c'est une habitude du bureau...)). Au final, si il ne sont pas appliqué, ça ne changera pas grand chose pour moi 😉 |
Hello @detobel36 Invitation envoyée ^^ Pas de souci, pour dire vrai le code a été fait avec des bouts de copier-coller car je n'avais jamais fait de plugin Kodi. Sinon j'avais oublié de répondre, en effet, c'est que sur Pastebin car j'avais vu la partie résolution. |
5e53330
to
57ae47a
Compare
Voila, j'ai rebase ta branche en corrigeant les soucis. Il faudra bien évidemment supprimé ce commit de fixup avant de merge. Commandes à faire (en étant sur la branche liée à cette PR): git rebase -i origin/Beta --autosquash
git push --force-with-lease |
Re @detobel36 Merci pour les modifs, je suppose que les commandes de merge ce n'est pas pour moi ? Bonne soirée |
Re, Ce sont des commandes qu'il faudra faire avant de merge, donc soit la personne qui fait le merge, le fait avant de merge. Bref, tu ne dois pas faire ces commandes. Et si besoin, je veux bien les faire moi 😃 Bonne journée |
Hello @detobel36 Parfait, je te fais confiance là-dessus et aussi pour les modifications ^ |
En très court ça permet de définir tous les termes dans les résolutions qu'on ne veut pas voir dans le "dossier" "BLACKLIST APPLIED".
J'ai remarqué que parfois, il faut appliquer 2 termes différents, par exemple "4K" et la version avec un "pixelP".
J'ai fait le patch que j'ai reporté sur la branch "Beta" mais par contre, je ne l'ai pas testé avec une installation de cette branche. Ça ne devrait rien changer normalement.