Skip to content
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

PWA 2020 chapter. #1613

Merged
merged 35 commits into from
Dec 6, 2020
Merged

PWA 2020 chapter. #1613

merged 35 commits into from
Dec 6, 2020

Conversation

hemanth
Copy link
Contributor

@hemanth hemanth commented Dec 2, 2020

Converted the shared document to markdown. Progress on #909

Converted the shared document to markdown.
@hemanth hemanth changed the title Final version in markdown. PWA 2020 chapter. Dec 2, 2020
@hemanth hemanth mentioned this pull request Dec 2, 2020
10 tasks
@hemanth
Copy link
Contributor Author

hemanth commented Dec 2, 2020

authors and reviewers section should be updated.

@rviscomi rviscomi added the writing Related to wording and content label Dec 2, 2020
@rviscomi rviscomi added this to the 2020 Content Writing milestone Dec 2, 2020
@hemanth
Copy link
Contributor Author

hemanth commented Dec 3, 2020

@rviscomi I have addressed the comments.

@rviscomi rviscomi requested a review from logicalphase December 3, 2020 03:13
@rviscomi rviscomi removed their assignment Dec 3, 2020
@rviscomi
Copy link
Member

rviscomi commented Dec 3, 2020

Adding @logicalphase for a review as a courtesy if he has the time, but absolutely no obligation.

The next step is for @bazzadp to convert the figures.

@logicalphase
Copy link
Contributor

Hey all, I'll have it done and commented by tomorrow. Thanks for your patience.

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the figures and staged the chapter here: https://20201204t022443-dot-webalmanac.uk.r.appspot.com/en/2020/pwa

I've also completed and initial review of the chapter and provided feedback.

@rviscomi rviscomi assigned hemanth and logicalphase and unassigned tunetheweb Dec 4, 2020
@hemanth
Copy link
Contributor Author

hemanth commented Dec 5, 2020

Thanks @bazzadp fixed them, the build is green too.

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have performance a minimal edit that I feel this needed for make the chapter launch-ready.

We still need to do a further edit after this, but may not have time for that before launch so wanted to do what is needed to allow us to be included in this week's launch.

@hemanth if you could review ASAP that would be much appreciated!

@hemanth
Copy link
Contributor Author

hemanth commented Dec 6, 2020

@bazzadp Done, thank you! [I guess few of the commits on the suggestions weren't applied]

I do see the message, Suggestion Applied but on refresh they aren't really applied!

hemanth and others added 3 commits December 6, 2020 09:10
@tunetheweb
Copy link
Member

@bazzadp Done, thank you! [I guess few of the commits on the suggestions weren't applied]

Still loads of hidden suggestions you haven't actioned. By the way use the "Add suggestions to batch" and then submit in one go - it's a lot faster!

image

@hemanth
Copy link
Contributor Author

hemanth commented Dec 6, 2020

@bazzadp Yes, just did, looks like in-line application is failing and new tab batch was better.

I did see the message: Suggestion Applied but on refresh they aren't really applied!

@tunetheweb
Copy link
Member

OK were still some more. I went ahead and committed them since you seemed to accept the others. Hope that's OK?

If the automated test pass I'll merge this.

@tunetheweb tunetheweb merged commit d2dfd32 into HTTPArchive:main Dec 6, 2020
@tunetheweb
Copy link
Member

Merged. Thanks for all your hard work @hemanth - we’ll come back for a round of edits later.

@logicalphase id you get any time to give any feedback or even do any writing then feel free to open a new PR for that. Sorry I didn’t want for you but time running out so needed to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
writing Related to wording and content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants