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

refactor: don't send ipcRenderer.sendSync() reply as array #19245

Closed
wants to merge 1 commit into from
Closed

refactor: don't send ipcRenderer.sendSync() reply as array #19245

wants to merge 1 commit into from

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Jul 13, 2019 •
edited
Loading

Description of Change

event.sendReply() was refactored to take base::Value instead of base::ListValue in #18449. We don't need to pass the return value of ipcRenderer.sendSync() as an array via event.sendReply() anymore.

Checklist

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

Release Notes

Notes: no-notes

@miniak miniak requested a review from nornagon July 13, 2019 19:12
@miniak miniak self-assigned this Jul 13, 2019
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jul 13, 2019
@miniak miniak added the wip ⚒ label Jul 13, 2019
@miniak miniak force-pushed the miniak/send-sync-reply branch from b405ff2 to e5d0069 Compare July 13, 2019 19:18
@miniak miniak removed the wip ⚒ label Jul 13, 2019
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jul 14, 2019
zcbenz
zcbenz reviewed Jul 15, 2019
View reviewed changes
Copy link
Contributor

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

The tests are failing.

@miniak miniak closed this Jul 15, 2019
@miniak miniak deleted the miniak/send-sync-reply branch July 15, 2019 08:54
@miniak
Copy link
Contributor Author

miniak commented Jul 15, 2019

@zcbenz it seems like I cannot be done as undefined cannot be transferred as plain base::Value, it needs to be done via an empty array

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

@zcbenz zcbenz zcbenz left review comments

@nornagon nornagon Awaiting requested review from nornagon

Assignees

@miniak miniak

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@miniak @zcbenz