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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document setPermissionRequestHandler(null) #11059

Merged
merged 3 commits into from Nov 21, 2017

Conversation

Projects
None yet
5 participants
@felixrieseberg
Member

felixrieseberg commented Nov 8, 2017

We never documented how to clear a permissions request handler, leaving developers believing that we forgot to find a way to remove 'em (#11057).

We didn't, we just never documented it 馃槈

Closes #11057

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Nov 8, 2017

You broke my typescript generation code 馃槩

馃槅 I'll take a look at fixing the generator 馃憤

EDIT: On a positive note our tests caught that something went wrong with the typescript file 馃挴

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Nov 8, 2017

@felixrieseberg PR's up to the parser and generator to handle this kind of situation 馃憤

@deepak1556

Thanks!

@felixrieseberg

This comment has been minimized.

Member

felixrieseberg commented Nov 10, 2017

Rebased onto master

@codebytere

This comment has been minimized.

Member

codebytere commented Nov 16, 2017

@felixrieseberg throw an npm run lint at this one and i'll get it merged 馃榿

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Nov 16, 2017

@codebytere This PR will break master until the upstream docs parser and typescript generator PR's get merged and released 馃憤

@codebytere

This comment has been minimized.

Member

codebytere commented Nov 16, 2017

ah gotcha! nevermind then haha

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Nov 20, 2017

Close and reopen to trigger the PRs.

@zcbenz zcbenz closed this Nov 20, 2017

@zcbenz zcbenz reopened this Nov 20, 2017

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Nov 20, 2017

@felixrieseberg Can you bump the versions of docs parser and typescript generator?

@felixrieseberg

This comment has been minimized.

Member

felixrieseberg commented Nov 20, 2017

Will do!

@felixrieseberg felixrieseberg requested a review from electron/reviewers as a code owner Nov 20, 2017

@felixrieseberg

This comment has been minimized.

Member

felixrieseberg commented Nov 20, 2017

Bumped electron-docs-linter, but electron-typescript-definitions needs a new update. I'll go and hassle @zeke.

@felixrieseberg

This comment has been minimized.

Member

felixrieseberg commented Nov 20, 2017

@zcbenz @MarshallOfSound Both updated, this tiny change could now finally land 馃槅

@MarshallOfSound

Assuming everything goes green, good to land this critical one-liner 馃槅

@zcbenz zcbenz merged commit 596a61f into master Nov 21, 2017

6 checks passed

ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@zcbenz zcbenz deleted the clear-permissions-handler branch Nov 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment