Issue 7205 - Change classes that use ALL_CAPS naming to be CamelCase

Issue 7205 - Change classes that use ALL_CAPS naming to be CamelCase

[rhowell, 2019-01-12 01:25:31]

Hi Vasily! Sorry to keep bugging you. ;)  Is there someone else I should be
tagging as a reviewer? Also, one question below. Thank you!

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.py
File tests/test_parser.py (right):

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.p...
tests/test_parser.py:22: SelectorType as St, FilterAction as Fa, FilterOption as
Opt,
I wasn't sure what best practices were for this naming scheme. St, Fa, and Opt
seem a little odd to me. Any suggestions?

[Vasily Kuznetsov, 2019-01-12 19:23:39]

Hi Rosie,

No worries about bugging me -- I'm the right reviewer for CMS code. Perhaps
Tudor could be a backup/co-reviewer. He probably wouldn't mind and he worked on
CMS a bit. Or maybe Jon...

There is just one small comment about comments (pun intended) and the answer to
your question below.

Cheers,
Vasily

https://codereview.adblockplus.org/29979570/diff/29979571/abp/filters/parser.py
File abp/filters/parser.py (right):

https://codereview.adblockplus.org/29979570/diff/29979571/abp/filters/parser....
abp/filters/parser.py:52: class SelectorType:  # flake8: noqa (this is a
namespace of constants).
You should be able to delete the comment that disables flake8 now, at least this
was the idea of this ticket :D

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.py
File tests/test_parser.py (right):

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.p...
tests/test_parser.py:22: SelectorType as St, FilterAction as Fa, FilterOption as
Opt,
On 2019/01/12 01:25:31, rhowell wrote:
> I wasn't sure what best practices were for this naming scheme. St, Fa, and Opt
> seem a little odd to me. Any suggestions?

Yeah, I agree about the oddness. Maybe just the first letters of the words in
CAPS: ST, FA and FO. How does this feel?

[rhowell, 2019-01-12 21:18:35]

https://codereview.adblockplus.org/29979570/diff/29979571/abp/filters/parser.py
File abp/filters/parser.py (right):

https://codereview.adblockplus.org/29979570/diff/29979571/abp/filters/parser....
abp/filters/parser.py:52: class SelectorType:  # flake8: noqa (this is a
namespace of constants).
On 2019/01/12 19:23:38, Vasily Kuznetsov wrote:
> You should be able to delete the comment that disables flake8 now, at least
this
> was the idea of this ticket :D

Done.

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.py
File tests/test_parser.py (right):

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.p...
tests/test_parser.py:22: SelectorType as St, FilterAction as Fa, FilterOption as
Opt,
On 2019/01/12 19:23:38, Vasily Kuznetsov wrote:

> Maybe just the first letters of the words in
> CAPS: ST, FA and FO. How does this feel?

That produces another flake8 error: `N814 camelcase 'FA' imported as constant`.
I tried a couple different options. SelectorType is the only one that causes the
lines to be too long, so I shortened it to SelType. But the rest of them could
just be imported as-is?

[Vasily Kuznetsov, 2019-01-14 14:14:35]

LGTM

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.py
File tests/test_parser.py (right):

https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.p...
tests/test_parser.py:22: SelectorType as St, FilterAction as Fa, FilterOption as
Opt,
On 2019/01/12 21:18:34, rhowell wrote:
> On 2019/01/12 19:23:38, Vasily Kuznetsov wrote:
> 
> > Maybe just the first letters of the words in
> > CAPS: ST, FA and FO. How does this feel?
> 
> That produces another flake8 error: `N814 camelcase 'FA' imported as
constant`.
> I tried a couple different options. SelectorType is the only one that causes
the
> lines to be too long, so I shortened it to SelType. But the rest of them could
> just be imported as-is?

I see. Pep8-naming is stricter than I expected. This is a bit verbose but I
don't really have better ideas :/

[rhowell, 2019-01-14 15:36:57]

On 2019/01/14 14:14:35, Vasily Kuznetsov wrote:
> LGTM
> 
> https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.py
> File tests/test_parser.py (right):
> 
>
https://codereview.adblockplus.org/29979570/diff/29979571/tests/test_parser.p...
> tests/test_parser.py:22: SelectorType as St, FilterAction as Fa, FilterOption
as
> Opt,
> On 2019/01/12 21:18:34, rhowell wrote:
> > On 2019/01/12 19:23:38, Vasily Kuznetsov wrote:
> > 
> > > Maybe just the first letters of the words in
> > > CAPS: ST, FA and FO. How does this feel?
> > 
> > That produces another flake8 error: `N814 camelcase 'FA' imported as
> constant`.
> > I tried a couple different options. SelectorType is the only one that causes
> the
> > lines to be too long, so I shortened it to SelType. But the rest of them
could
> > just be imported as-is?
> 
> I see. Pep8-naming is stricter than I expected. This is a bit verbose but I
> don't really have better ideas :/

Gotcha. I guess I'll push it then.  :)