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.

Sign up for GitHub

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

Jump to bottom

Permissions API permissions descriptions linter #6156

Merged
merged 3 commits into from
May 20, 2020
Merged

Permissions API permissions descriptions linter #6156

merged 3 commits into from
May 20, 2020

Conversation

bershanskiy
Copy link
Contributor

Fixes #5911.

  • Document existing convention for naming Permissions API permissions
  • Add linter to enforce this convention
  • Fix typo in "permissons" caught by linter

I have no idea how I made a typo "permissons" in the original #5909 and it slipped through the review, but that perfectly demonstrates the need for linter.

@bershanskiy bershanskiy requested a review from Elchi3 as a code owner May 14, 2020 21:43
@ghost ghost added data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API docs Issues or pull requests regarding the documentation of this project. linter Issues or pull requests regarding the tests / linter of the JSON files. labels May 14, 2020
bershanskiy
bershanskiy commented May 14, 2020
View reviewed changes
api/Permissions.json Outdated Show resolved Hide resolved
@queengooborg
Copy link
Contributor

Thanks for the PR! This linter will be great to have.

The API descriptions should be pretty easy to review and merge, and we can get those in without a linter. Can we separate this into two PRs: one with the linter and one with the data? (I can then review the data updates and get those merged right away.)

@bershanskiy bershanskiy mentioned this pull request May 15, 2020
Merged
@bershanskiy
Permissions API permissions descriptions linter
5dbbc9f 
 - Document existing convention for naming Permissions API permissions
 - Add linter to enforce this convention
 - Fix typo in "permissons" caught by linter
@bershanskiy
Copy link
Contributor Author

I'll rebase this PR to finish splitting off data changes merged in #6162.

@queengooborg queengooborg removed the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label May 15, 2020
ddbeck
ddbeck requested changes May 20, 2020
View reviewed changes
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Good idea! Some doc suggestions in line comments.

docs/data-guidelines.md Outdated Show resolved Hide resolved
docs/data-guidelines.md Show resolved Hide resolved
docs/data-guidelines.md Show resolved Hide resolved
@ddbeck ddbeck self-requested a review May 20, 2020 16:55
ddbeck
ddbeck approved these changes May 20, 2020
View reviewed changes
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Excellent. Thank you! 🎉

@ddbeck ddbeck merged commit fcf93f9 into mdn:master May 20, 2020
@bershanskiy bershanskiy deleted the linter-permissions-api-descriptions branch May 20, 2020 19:11
@Elchi3 Elchi3 removed their request for review May 21, 2020 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddbeck ddbeck ddbeck approved these changes

Assignees
No one assigned
Labels
docs Issues or pull requests regarding the documentation of this project. linter Issues or pull requests regarding the tests / linter of the JSON files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add "description"s guidelines for Permissions API descriptions
3 participants
@bershanskiy @queengooborg @ddbeck