Project standardization #6
Conversation
bandophahita
force-pushed
the
project_standardization
branch
from
afdf31d
to
7c6ff43
Compare
screenpy_playwright/protocols.py
Outdated
|
|
||
|
|
||
| __all__ = ["PageObject"] |
There was a problem hiding this comment.
I think this was maybe supposed to be in the __init__.py?
There was a problem hiding this comment.
This one was odd because we do from .protocols import * in __init__.py. I figured we could either do individual imports in the __init__ or just add an __all__ to protocols.py.
This felt like less work in the future should any new protocols get added. /shrug
There was a problem hiding this comment.
Hm... i'm kinda surprised it works. I don't know why it wouldn't, i just never thought to try putting __all__ in a non-__init__ file.
It doesn't follow the pattern we've got on the other projects though. I think it's weirder to have an __all__ in here than it is to import each piece in __init__.py.
Though... i guess i don't know how common it is to put an __all__ in a file like this. Maybe this is the standard we should be following in the other projects?
There was a problem hiding this comment.
I can go either way, tbh. I'm not sure I have a preference.
The use of __all__ in a file is a little of a double edged sword. It's typically only meant for clean from module import * which the community frowns on a little. That said, when you have a legitimate need for it, it keeps your namespace clean.
That said, I agree that keeping them uniform makes sense. I'm currently leaning towards path of least effort, which would update the __init__.py to follow the pattern from the other packages.
There was a problem hiding this comment.
I updated it to follow the pattern.
|
I'm gonna just merge this in 'cause i approve it and i need this stuff for my PR! |
ScreenPyHQ/screenpy#127
Attempting to make various portions of each screenpy project uniform.