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

fix: some typos in permissions #3436

Merged
merged 1 commit into from
Dec 3, 2019
Merged

fix: some typos in permissions #3436

merged 1 commit into from
Dec 3, 2019

Conversation

afinch7
Copy link
Contributor

@afinch7 afinch7 commented Dec 3, 2019

I'm pretty sure these where mistakes, but not 100% sure here.

cc @kt3k

kt3k
kt3k approved these changes Dec 3, 2019
View reviewed changes
Copy link
Member

@kt3k kt3k left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@afinch7 Thanks! Those are (terrible) mistakes!

@afinch7
Copy link
Contributor Author

afinch7 commented Dec 3, 2019

Any idea why this wasn't causing tests to fail on CI. It was on my machine.

@kt3k
Copy link
Member

kt3k commented Dec 3, 2019 •
edited
Loading

@afinch7 The assertions were too simple in test_permission_request_** test cases. Asserting other permissions' states should prevent this error. For example, adding assert_eq!(perms1.allow_run, PermissionState::Ask); at line 619 prevents allow_net typo happing again.

ry
ry approved these changes Dec 3, 2019
View reviewed changes
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - good catch @afinch7

@ry ry merged commit 00844b4 into denoland:master Dec 3, 2019
bartlomieju pushed a commit to bartlomieju/deno that referenced this pull request Dec 28, 2019
@afinch7 @bartlomieju
fix some typos in permissions (denoland#3436)
4850195
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ry ry ry approved these changes

@kt3k kt3k kt3k approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@afinch7 @kt3k @ry