Issue 2822 - Added support for new subscription types

Issue 2822 - Added support for new subscription types

[Sebastian Noack, 2015-08-31 11:41:06]

https://codereview.adblockplus.org/29325371/diff/29325372/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29325371/diff/29325372/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:204: if self.recommendation !=
None and self.type == 'ads' and (self.languages == None or not re.search(r'\S',
self.languages)):
  not re.search(r'\S', self.languages)

.. is equivalent to ..

  self.languages.strip()

https://codereview.adblockplus.org/29325371/diff/29325372/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:204: if self.recommendation !=
None and self.type == 'ads' and (self.languages == None or not re.search(r'\S',
self.languages)):
Also please use |is None| or |is not None| instead ==/!= when comparing to None.

[Thomas Greiner, 2015-08-31 15:27:47]

https://codereview.adblockplus.org/29325371/diff/29325372/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29325371/diff/29325372/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:204: if self.recommendation !=
None and self.type == 'ads' and (self.languages == None or not re.search(r'\S',
self.languages)):
On 2015/08/31 11:41:06, Sebastian Noack wrote:
> Also please use |is None| or |is not None| instead ==/!= when comparing to
None.

Done.

https://codereview.adblockplus.org/29325371/diff/29325372/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:204: if self.recommendation !=
None and self.type == 'ads' and (self.languages == None or not re.search(r'\S',
self.languages)):
On 2015/08/31 11:41:06, Sebastian Noack wrote:
>   not re.search(r'\S', self.languages)
> 
> .. is equivalent to ..
> 
>   self.languages.strip()

Done but I kept the "not" to achieve the same results because the regular
expression is checking for non-whitespaces.

[Sebastian Noack, 2015-08-31 15:53:44]

https://codereview.adblockplus.org/29325371/diff/29325397/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29325371/diff/29325397/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:196: if self.type not in ['ads',
'anti-adblock', 'other', 'malware', 'social', 'privacy']:
Nit: Please use a tuple. There is no reason to use a mutable data structure
here.

https://codereview.adblockplus.org/29325371/diff/29325397/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:204: if self.recommendation is
not None and self.type == 'ads' and (self.languages is None or not
self.languages.strip()):
There are two ways to further simplify that check:

  not (self.languages || '').strip()

.. or ..

  not (self.languages and self.languages.strip())

[Thomas Greiner, 2015-09-02 14:10:19]

https://codereview.adblockplus.org/29325371/diff/29325397/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29325371/diff/29325397/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:196: if self.type not in ['ads',
'anti-adblock', 'other', 'malware', 'social', 'privacy']:
On 2015/08/31 15:53:44, Sebastian Noack wrote:
> Nit: Please use a tuple. There is no reason to use a mutable data structure
> here.

Done.

https://codereview.adblockplus.org/29325371/diff/29325397/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:204: if self.recommendation is
not None and self.type == 'ads' and (self.languages is None or not
self.languages.strip()):
On 2015/08/31 15:53:44, Sebastian Noack wrote:
> There are two ways to further simplify that check:
> 
>   not (self.languages || '').strip()
> 
> .. or ..
> 
>   not (self.languages and self.languages.strip()) 

Done.

[Sebastian Noack, 2015-09-02 15:00:34]

LGTM