web.dev icon indicating copy to clipboard operation
web.dev copied to clipboard

Fix code example

Open polotek opened this issue 3 years ago • 5 comments

Changes proposed in this pull request:

Minor content fix for correctness. The previous code example ran someFunction immediately instead of inside the promise callback.

When you're ready to submit your PR, don't forget to add the $-presubmit label.

polotek avatar Mar 17 '22 09:03 polotek

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

google-cla[bot] avatar Mar 17 '22 09:03 google-cla[bot]

Adding @housseindjirdeh who wrote the article and @malchata for performance expertise -- if one of you confirms, I'll merge

alexandrascript avatar Mar 21 '22 13:03 alexandrascript

Thanks for catching this @polotek! All looks good to me. Only thing is we'll need you to sign the CLA before the workflow will allow us to merge your contribution.

malchata avatar Mar 21 '22 15:03 malchata

Hello! This is an automated review by our custom reviewbot. It updates automatically when code or GitHub comments in this pull request are created or updated.

Requested changes

If there are any common problems with the content files you created or modified, they will be listed here.

src/site/content/en/fast/reduce-javascript-payloads-with-code-splitting/index.md

  • This file passed all of our automated Markdown audits.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. To prevent this from happening, leave a comment.

stale[bot] avatar Jun 19 '22 16:06 stale[bot]

Hi @polotek! Are you still working on this fix?

If not, I'm willing to take over and create an issue and PR to fix the code so we can improve the documentation

@heyawhite @housseindjirdeh @malchata waiting for your response to make my contribution

miguelfdezc avatar Aug 19 '22 14:08 miguelfdezc

@miguelfdezc did you sign the CLA? If not, please do so we can merge this.

malchata avatar Aug 22 '22 14:08 malchata

Hi @malchata, I did sign the CLA. I believe you wanted to tag @polotek instead? Actually, the reason why I commented was that I wanted to fix this too but I was there was this PR already but it was stale because of the missing CLA sign

miguelfdezc avatar Aug 22 '22 15:08 miguelfdezc

@polotek, if you can sign the CLA, I can merge this change. Sorry for the hassle!

malchata avatar Aug 22 '22 15:08 malchata

@miguelfdezc, feel free to go ahead and open another PR with the fix while mentioning this one 🙂

matthiasrohmer avatar Aug 24 '22 12:08 matthiasrohmer

Superseded by #8589.

matthiasrohmer avatar Aug 24 '22 13:08 matthiasrohmer

Thank you for trying to get in touch to give me full attribution. I have now signed the CLA for future reference. But I'm fine with how this ticket was resolved.

polotek avatar Sep 10 '22 21:09 polotek