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

Add ServiceWorkerGlobalScope.serviceWorker #29461

Merged
merged 23 commits into from
Dec 18, 2023
Merged

Conversation

skyclouds2001
Copy link
Contributor

@skyclouds2001 skyclouds2001 commented Oct 4, 2023

Description

add ServiceWorkerGlobalScope.serviceWorker documentation

Motivation

Additional details

https://w3c.github.io/ServiceWorker/#serviceworkerglobalscope-interface

https://w3c.github.io/ServiceWorker/#service-worker-global-scope-serviceworker

Related issues and pull requests

Fixes #29123
Fixes #29541

@github-actions github-actions bot added the Content:WebAPI Web API docs label Oct 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

@skyclouds2001 skyclouds2001 marked this pull request as ready for review October 4, 2023 10:42
@skyclouds2001 skyclouds2001 requested a review from a team as a code owner October 4, 2023 10:42
@skyclouds2001 skyclouds2001 requested review from wbamberg and removed request for a team October 4, 2023 10:42
@hamishwillee
Copy link
Collaborator

Flyby comment,

@skyclouds2001
Copy link
Contributor Author

skyclouds2001 commented Oct 6, 2023

Flyby comment,

For I, I think as it is a simple getter, maybe it is not quite necessary to provide an example

Also, as ServiceWorkerGlobalScope.registration and ServiceWorkerGlobalScope.clients do not provide an example as the same as ServiceWorkerGlobalScope.serviceWorker. If it is necessary to add an exaple, I think it is better to do this in another pull request

For II, I'll create another pull request to add the additonal content to ServiceWorker documentation, see #29541

@hamishwillee your idea?

@hamishwillee
Copy link
Collaborator

hamishwillee commented Oct 8, 2023

For I, I think as it is a simple getter, maybe it is not quite necessary to provide an example

I've had reviewers ask for this of me because readers like to see an example - it makes copy-paste easy and confirms that the code to use the API will indeed be what they think it is.

As a fly-by reviewer I'm explicitly removing myself as the person that places requirements on you - I'm just adding comments. BUT, if you do add an example it should be done in this PR and so should the ServiceWorker docs - either it is related an important and it should be done as the same work, or it isn't and can be ignored for the PR. But creating another PR just increases the chance it doesn't get done.

skyclouds2001 and others added 3 commits October 10, 2023 01:47
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@skyclouds2001
Copy link
Contributor Author

As #29451 seems to not be working, that issue will fix in this pull request

@skyclouds2001
Copy link
Contributor Author

For I, I think as it is a simple getter, maybe it is not quite necessary to provide an example

I've had reviewers ask for this of me because readers like to see an example - it makes copy-paste easy and confirms that the code to use the API will indeed be what they think it is.

As a fly-by reviewer I'm explicitly removing myself as the person that places requirements on you - I'm just adding comments. BUT, if you do add an example it should be done in this PR and so should the ServiceWorker docs - either it is related an important and it should be done as the same work, or it isn't and can be ignored for the PR. But creating another PR just increases the chance it doesn't get done.

Thanks for your suggestion!

@hamishwillee
Copy link
Collaborator

@wbamberg If you don't look this week I will try to next week.

@skyclouds2001
Copy link
Contributor Author

cc @hamishwillee would you please review this if available

hamishwillee and others added 2 commits December 18, 2023 11:03
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks @skyclouds2001 . I removed the Syntax sections - not used as that is "Value" for properties. Decided you were right - we can live without an example for a trivial getter. Merging.

@hamishwillee hamishwillee merged commit 0778b4f into mdn:main Dec 18, 2023
7 checks passed
@hamishwillee
Copy link
Collaborator

PS Sorry for the delay. Finally!

@skyclouds2001 skyclouds2001 deleted the patch-1 branch December 18, 2023 02:56
dipikabh pushed a commit to dipikabh/content that referenced this pull request Jan 17, 2024
* Update index.md

* Create index.md

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Apply suggestions from code review

* fix

* Apply suggestions from code review

* Update files/en-us/web/api/serviceworkerglobalscope/clients/index.md

* Update files/en-us/web/api/serviceworker/index.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update files/en-us/web/api/serviceworkerglobalscope/registration/index.md

* Update files/en-us/web/api/serviceworkerglobalscope/serviceworker/index.md

* Update files/en-us/web/api/serviceworkerglobalscope/serviceworker/index.md

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hamish Willee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs
Projects
None yet
2 participants