scrapinghub / web-poet

Web scraping Page Objects core library
https://web-poet.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
95 stars 15 forks source link

Refactor the rules documentation #169

Closed Gallaecio closed 1 year ago

Gallaecio commented 1 year ago

My goal was to greatly simplify https://web-poet.readthedocs.io/en/stable/page-objects/rules.html.

However, I wonder if I went too far; the new pages are ~25% of the old page in terms of line count.

I removed much of the content, and although I aimed to keep the core information around, I did not always do that. For example, the entire last section is gone without its information making it in any form into the new page; I wonder if we should keep some of that information.

I recommend reading the old page, then the new pages, and see how you feel about the missing information.

codecov[bot] commented 1 year ago

Codecov Report

Merging #169 (2e9a9c0) into master (bfd6683) will increase coverage by 0.00%. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #169 +/- ## ======================================= Coverage 98.59% 98.60% ======================================= Files 30 30 Lines 1356 1362 +6 ======================================= + Hits 1337 1343 +6 Misses 19 19 ``` | [Impacted Files](https://app.codecov.io/gh/scrapinghub/web-poet/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapinghub) | Coverage Δ | | |---|---|---| | [web\_poet/rules.py](https://app.codecov.io/gh/scrapinghub/web-poet/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapinghub#diff-d2ViX3BvZXQvcnVsZXMucHk=) | `100.00% <ø> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/scrapinghub/web-poet/pull/169/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapinghub)
wRAR commented 1 year ago

Not sure if completely removing docs is a good idea in general but I've found the old version of the page to be very complicated, especially for someone who is not familiar with scrapy-poet (when reading it back then) yet so trimming it down is welcome.

kmike commented 1 year ago

Hey @Gallaecio! These docs look much better than before, so I'm merging it. I still think though that we should improve the introduction: the "output class" thing doesn't look clear to me, but I don't have a concrete suggestion.