-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
2020 Resource Hints chapter #1587
Conversation
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 quick suggestion to get the build working
I've staged the draft here to assist with the review: https://20201127t212713-dot-webalmanac.uk.r.appspot.com/en/2020/resource-hints
Looks good to me @Zizzamia but difficult to be sure until we see the charts in place. I would normally advocate for smaller PRs and an iterative approach but, since the chapter isn't actually launch-able without the charts I think we should change this to a Draft PR, and then hold it open until the charts are added and the TODOs are finished out. BTW, just a heads up, after we merge this, there will be a round of editing from someone outside the immediate team (@rviscomi or myself most likely unless we can get some more volunteer Editors) 😁 to try to bring a consistent style and voice across all the chapters. This can be substantial just to warn you as some Authors are surprised with the level of editing. You'll will be asked to review all suggested edits to make sure you are comfortable with them. It is possible to launch the chapter with an unedited tag (see the 2020 Markup Chapter as an example), but that is the minimal viable product of the chapter so do really need the images to get to that: |
reviewers: [notwillk, giopunt, jessnicolet, pmeenan, mgechev] | ||
analysts: [khempenius] |
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.
Can you verify that all of these reviewers/analysts have meaningfully contributed to the chapter and are roughly in sorted order so that the biggest contributors are listed first?
Co-authored-by: Rick Viscomi <[email protected]>
Co-authored-by: Rick Viscomi <[email protected]>
Co-authored-by: Rick Viscomi <[email protected]>
Co-authored-by: Rick Viscomi <[email protected]>
Co-authored-by: Rick Viscomi <[email protected]>
@bazzadp thank you for the initial push-back, with @rviscomi we added the charts into this PRs. Also we were able to update the order of reviewers based on the amount of contributions and added all the correct If it's ok for the team I will follow up with more polishing figures, backlinks review and possible final editing in the next PRs. And yeah, looking forward from all editors to review and re-shape the article in a way it's more aligned with the rest of the Almanac 2020. 🌲🌕🌲 |
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.
Very well done @Zizzamia ! Great chapter and I enjoyed reading that.
I do have some comments for you to consider though.
@@ -5,19 +5,367 @@ chapter_number: 21 | |||
title: Resource Hints | |||
description: Resource Hints chapter of the 2020 Web Almanac covering usage of dns-prefetch, preconnect, preload, prefetch, priority hints, and native lazy loading. | |||
authors: [Zizzamia] | |||
reviewers: [notwillk, giopunt, jessnicolet, pmeenan, mgechev] | |||
reviewers: [jessnicolet, pmeenan, giopunt, mgechev, notwillk] |
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.
@notwillk can you add yourself to the 2020,json config so you will render properly?
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.
It looks like I'm in there (name is wrong). Is up dating my name all I need to do there?
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.
{{ figure_markup( | ||
caption="Percentage of pages using native lazy loading set to “lazy”", | ||
content="3.87%", | ||
classes="big-number", | ||
sheets_gid="2039808014", | ||
sql_file="native_lazy_loading_attrs.sql" | ||
) }} |
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.
Is this images or images and iframes? Think it would be good to clarify (applies to this whole section). And if just images should we have a sentence or two on iframes? We use it on our site :-)
Oh and one more comment, there do seem to be an awful lot of the big callout figures in this chapter. Very much like resource hints, over use can diminish the impact of all of them 😁 Could we replace any with charts or tables? |
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
@bazzadp thank you for reviewing, and yeah I would love to replace a few figures with tables and even add a bit more code examples. Would you like I made all future changes in this PR on in the next one (including a few comments you left as well)? |
I'd like to see us merge and iterate to unblock the editing step. If there's anything else that needs to be changed please leave TODO's in the markdown and resolve as suggestions in the editing PR itself, or a follow-up PR. |
Agreed. @Zizzamia could you have one last pass through all the open comments and either resolve them or add a Jinja2 comment (using
We can then edit this chapter and come back to these few remaining points later. |
Co-authored-by: Barry Pollard <[email protected]>
…chive/almanac.httparchive.org into zizzamia/init-resource-hints-2020
Co-authored-by: Barry Pollard <[email protected]>
…chive/almanac.httparchive.org into zizzamia/init-resource-hints-2020
Ok great! |
In this PR we introduce the first official draft as markdown for the Resource Hints 2020 chapter #920.
Following PRs will focus on:
Thank you @rviscomi and @OBTo for all the guidance, and all reviewers and analysts (@notwillk, @giopunt, @jessnicolet, @pmeenan, @mgechev) for the incredible feedbacks and help. 🙏
Please, feel free to keep mentioning anything in terms of content or data we might want to polish, I am going to use the next few days to do several final clean-up anyway. 🌲 🚀 🌕