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

refactor: convert ipc-renderer.ts to TypeScript #19556

Merged
merged 2 commits into from Aug 2, 2019

Conversation

@miniak
Copy link
Contributor

commented Jul 31, 2019

Description of Change

Convert ipc-renderer.ts to TypeScript. Also make the ipc binding typesafe.

Note: there is an issue with ipcRenderer.sendToAll, which is not documented. We can either remove it or document it.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • PR title follows semantic commit guidelines

Release Notes

Notes: Removed undocumented ipcRenderer.sendToAll().

@miniak miniak requested review from codebytere and MarshallOfSound Jul 31, 2019

@miniak miniak self-assigned this Jul 31, 2019

@miniak miniak force-pushed the miniak/ipc-renderer-ts branch 2 times, most recently from 6d71dab to 6beef3b Jul 31, 2019

@miniak miniak requested a review from nornagon Aug 1, 2019

@miniak miniak force-pushed the miniak/ipc-renderer-ts branch from 6beef3b to 09763ff Aug 1, 2019

@nornagon
Copy link
Contributor

left a comment

seems fine to me to remove undocumented APIs. Maybe note it in the release notes?

@MarshallOfSound
Copy link
Member

left a comment

I think it's ok to remove this API, it was never documented and was only ever made for internal use 🤷‍♂

typings/internal-ambient.d.ts Outdated Show resolved Hide resolved

@electron-cation electron-cation bot removed the new-pr 🌱 label Aug 1, 2019

Update typings/internal-ambient.d.ts
Co-Authored-By: Samuel Attard <sattard@slack-corp.com>

@ckerr ckerr merged commit 3011a0f into master Aug 2, 2019

13 checks passed

Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190802.6 succeeded
Details
electron-arm64-testing Build #20190802.6 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Aug 2, 2019

Release Notes Persisted

Removed undocumented ipcRenderer.sendToAll().

@miniak miniak deleted the miniak/ipc-renderer-ts branch Aug 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.