-
Notifications
You must be signed in to change notification settings - Fork 85
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
feat: third lesson about DevTools #1321
base: master
Are you sure you want to change the base?
Conversation
4bd5048
to
5453900
Compare
In 94b26f5 I added ![]() |
I gave up on extracting links here as this is a topic which is covered later already and if I covered it here I'd need to rewrite later lessons for no good reason.
94b26f5
to
a2f4466
Compare
Hey guys, I'm sorry, but I find it really hard to find the time for the reviews. If @metalwarrior665 can't do them reasonably fast, then we need to find someone else to chip in. Perhaps @souravjain540? It's a shame that this great content gets stuck here for so long. |
I didn't know we were even blocking these PRs on reviews. I have done some work in the JS space recently and I would definitely want to review any change there. For Python I think @souravjain540 would make sense |
guys, I am sorry I can't promise to give my best reviews in such a busy schedule right now, considering there are really high priority time bounded projects like: the OS fair share program, PyCon Namibia: Workshop and Talk, Actor Bounty Program, PPE, and all my regular content tasks of Crawlee and others. I can come back to give reviews after PyCon Namibia if that's okay. |
As of now I'm in no rush with this one, I got stuck on other stuff. |
I could've sworn that it was already set up like this. But it also should omit frontmatter and in one document it insisted to check frontmatter and throw error so ¯\_(ツ)_/¯ |
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.
One typo and one suggestion otherwise looks good from my point of view
|
||
 | ||
|
||
We could either rely on the fact that the sale price is likely to be always the one which is highlighted, or that it's always the first price. For now we'll rely on the later and we'll let `querySelector()` to simply return the first result: |
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.
We could either rely on the fact that the sale price is likely to be always the one which is highlighted, or that it's always the first price. For now we'll rely on the later and we'll let `querySelector()` to simply return the first result: | |
We could either rely on the fact that the sale price is likely to be always the one which is highlighted, or that it's always the first price. For now we'll rely on the latter and we'll let `querySelector()` to simply return the first result: |
price.textContent; | ||
``` | ||
|
||
It works, but the price isn't alone in the result. Before we'd use such data, we'd need to do some **data cleaning**: |
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.
This sentence Before we'd...
somehow feels unnatural, perhaps something like this? Also within docs I try to keep bold text only to indicate UI elements and cursive as emphasis. And it seems like here bold is used as both emphasis & to highlight UI elements, isn't that a bit confusing?
It works, but the price isn't alone in the result. Before we'd use such data, we'd need to do some **data cleaning**: | |
It works, but the price isn't alone in the result. Before using such data, we'd need to do some _data cleaning_: |
Porting this lesson and this lesson: