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 proposal processes #543

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Add proposal processes #543

merged 2 commits into from
Mar 7, 2024

Conversation

rdcronin
Copy link
Contributor

Add a new proposal process for API changes.

This includes:

  • Documentation on how the proposal process works, and
  • A template to use when creating a new API proposal.

This should be used for future significant API modifications or additions. This should only be used when there is a browser that has an intent to implement the API change in the near future. This does not replace the process of filing issues for initial discussions around API changes or requests for new features.

More details are included in proposal_process.md.

Add a new proposal process for API changes. This includes a doc describing
the process and a template from which proposals may be made.
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

I added some suggestions and a question, but overall the process looks good.

Let's also add a proposals/README.md file that links the two new documents, for better visibility.

proposals/proposal_process.md Outdated Show resolved Hide resolved
proposals/proposal_process.md Outdated Show resolved Hide resolved
of a suggestion in a filed issue. As such, these proposals should only be
used for APIs that have support and commitment to implement from at least
one browser.
* Fill out the API Proposal. There are instructions in each section. For any
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Fill out the API Proposal. There are instructions in each section. For any
* Copy the [API Proposal template](proposal_template.md) and fill it out.
There are instructions in each section of the proposal template. For any

Added link to the template.

**Sponsoring Browser:** The browser vendor committed to implementing this API
if approved.

**Contributors:** <other contributors @>
Copy link
Member

Choose a reason for hiding this comment

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

What is the @ referring to? Would be nice to suggest the Github handle if any, or any other form of identification (whether an email, or a profile on the codereview site).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think Github handle makes the most sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased.

Comment on lines 47 to 49
resolved directly in the renderer must be asynchronous, and even if
something can be done in the renderer today, it should frequently be
asynchronous to prevent future breakage if we move something to the browser
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
resolved directly in the renderer must be asynchronous, and even if
something can be done in the renderer today, it should frequently be
asynchronous to prevent future breakage if we move something to the browser
resolved directly in a child process must be asynchronous, and even if
something can be done in the child process today, it should frequently be
asynchronous to prevent future breakage if we move something to the parent

"renderer"/"browser" is Chromium-specific terminology.

I replaced it with the more general "child"/"parent" process terminology. Instead of "parent", "different" could also be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased, and also added details that extensions usually run in child processes.

| New Permission 1 | Add a proposed warning string for the permission. If there should be no warning, provide justification as to why. Browser vendors can ultimately choose if there should be a warning and what it should be independently, but this can be useful to define especially if it has implications for the likelihood this proposal will succeed or be useful to developers. |

Document any new permission added by this API. Permissions are frequently the
same as the API itself, e.g. the browser.storage API has the permission
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
same as the API itself, e.g. the browser.storage API has the permission
same as the API itself, e.g. the `browser.storage` API has the permission

proposals/proposal_template.md Show resolved Hide resolved
one browser.
* Fill out the API Proposal. There are instructions in each section. For any
section that is not relevant, you may put "N/A" (likely with a brief summary
of why the section is not relevant, if it is not immediately obvious).
Copy link
Member

Choose a reason for hiding this comment

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

The template is very detailed with its instructions. Is the author supposed to delete the instructions make the doc more concise? If yes, let's emphasize that here (and maybe also in the template)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's the plan. Added a note here and will do so in the template as well.

proposal should be viewed as a possible "next step" from a suggested change or
developer issue and should imply that one or more browser vendors are actively
or imminently going to begin implementation. Most feature requests and bug
reports should continue to be reported as github issues in the WECG repository
Copy link
Collaborator

Choose a reason for hiding this comment

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

GitHub

Copy link
Contributor Author

@rdcronin rdcronin left a comment

Choose a reason for hiding this comment

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

Comments addressed. I'll add the README in the proposal directory in a separate commit just to keep this one targeted.

proposals/proposal_process.md Outdated Show resolved Hide resolved
one browser.
* Fill out the API Proposal. There are instructions in each section. For any
section that is not relevant, you may put "N/A" (likely with a brief summary
of why the section is not relevant, if it is not immediately obvious).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's the plan. Added a note here and will do so in the template as well.

**Sponsoring Browser:** The browser vendor committed to implementing this API
if approved.

**Contributors:** <other contributors @>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased.

Comment on lines 47 to 49
resolved directly in the renderer must be asynchronous, and even if
something can be done in the renderer today, it should frequently be
asynchronous to prevent future breakage if we move something to the browser
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased, and also added details that extensions usually run in child processes.

@Rob--W Rob--W merged commit 29136ba into w3c:main Mar 7, 2024
3 checks passed
github-actions bot added a commit that referenced this pull request Mar 7, 2024
SHA: 29136ba
Reason: push, by Rob--W

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants