-
Notifications
You must be signed in to change notification settings - Fork 15
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
move some registry functionalities from scrapy-poet #112
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #112 +/- ##
==========================================
- Coverage 99.85% 99.56% -0.30%
==========================================
Files 22 25 +3
Lines 697 921 +224
==========================================
+ Hits 696 917 +221
- Misses 1 4 +3
|
b699bd8
to
a59bd21
Compare
Co-authored-by: Mikhail Korobov <kmike84@gmail.com>
docs/page-objects/rules.rst
Outdated
|
||
from web_poet import default_registry | ||
|
||
item_cls = default_registry.page_object_for_item( |
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.
shouldn't it be page_cls?
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.
Do you mean renaming it something like page_object_cls_for_item
? or maybe page_cls_for_item
?
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.
The varible is named item_cls
, but it contains page class, not item class.
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.
I totally missed that. Thanks! Updated. 1ab1a43
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.
You actually raised a good point - should we rename this method to page_cls_for_item, as it doesn't return an instantiated object?
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.
Yeah, I think that's much better. e86bf37
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.
👍
Thanks @BurnzZ! |
Taken from scrapinghub/scrapy-poet#103.
TODO: