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

Fix permissions for management API #78

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Conversation

oliverdunk
Copy link
Member

A recent Chromium change [1] exposed the management API to the Chrome Web Store for testing purposes. Unfortunately, this meant chrome-types was thinking that the API was available without the management permission.

Unfortunately, I couldn't see an easy way to fix this.

To resolve that, I've made two tweaks here as workarounds:

  1. Ignore feature definitions that are only for webpages. We don't really ever have a reason to show those in our documentation.

  2. Teach the tool that if no channel is specified, this is equivalent to an API being available on stable.

Together, these seem to be enough to cause the tool to filter out the wrong definitions and pick up the ones used by most developers.

[1] https://source.chromium.org/chromium/chromium/src/+/59d0c94348743ce1831a113b81fd5dc420d68919:extensions/common/api/_api_features.json;dlc=30d48f6ef398e61601b43bcea809ac846fb99535

A recent Chromium change [1] exposed the management API to the Chrome
Web Store for testing purposes. Unfortunately, this meant chrome-types
was thinking that the API was available without the `management`
permission.

Unfortunately, I couldn't see an easy way to fix this.

To resolve that, I've made two tweaks here as workarounds:

1) Ignore feature definitions that are only for webpages. We don't
really ever have a reason to show those in our documentation.

2) Teach the tool that if no channel is specified, this is equivalent to
an API being available on stable.

Together, these seem to be enough to cause the tool to filter out the
wrong definitions and pick up the ones used by most developers.

[1] https://source.chromium.org/chromium/chromium/src/+/59d0c94348743ce1831a113b81fd5dc420d68919:extensions/common/api/_api_features.json;dlc=30d48f6ef398e61601b43bcea809ac846fb99535
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 44.47%. Comparing base (5659a0b) to head (3afa165).

Files Patch % Lines
tools/lib/feature-query.js 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   44.40%   44.47%   +0.07%     
==========================================
  Files          21       21              
  Lines        4076     4085       +9     
  Branches      284      287       +3     
==========================================
+ Hits         1810     1817       +7     
- Misses       2266     2268       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oliverdunk oliverdunk force-pushed the management-permissions-fix branch from ab0ceb4 to 642c255 Compare July 26, 2024 07:58
@oliverdunk oliverdunk force-pushed the management-permissions-fix branch from 642c255 to 3afa165 Compare July 26, 2024 07:59
@oliverdunk oliverdunk merged commit de60134 into main Jul 29, 2024
6 checks passed
@oliverdunk oliverdunk deleted the management-permissions-fix branch July 29, 2024 16:20
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.

2 participants