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

fix: crashReporter.getParameters() takes no params #17459

Merged
merged 1 commit into from Mar 28, 2019

Conversation

Projects
None yet
4 participants
@codebytere
Copy link
Member

codebytere commented Mar 19, 2019

Description of Change

Fixes an issue in crashReporter whereby getParameters took key and val as parameters and should take none. Also cleans up crashReporter.start(options) with default parameters.

Checklist

Release Notes

Notes: none

@MarshallOfSound
Copy link
Member

MarshallOfSound left a comment

It's not that I don't trust this and this change doesn't look small and simple but can we hold off on this till my updated crash reporter specs have landed. (I'm working on them right now).

Currently the crash reporter specs are completely disabled on linux and only half enabled on mac so I have very low confidence in CI for crash reporter changes atm 😆

@electron-cation electron-cation bot removed the new-pr 🌱 label Mar 20, 2019

@ckerr

This comment has been minimized.

Copy link
Member

ckerr commented Mar 23, 2019

@MarshallOfSound have those new crash reporter specs landed yet? If not, could you link to them in this PR so it's easy to see when this can land?

@miniak

miniak approved these changes Mar 27, 2019

@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Mar 27, 2019

@codebytere this does not seem to be a user facing change, unless you check crashReporter. getParameters.length

@MarshallOfSound

This comment has been minimized.

Copy link
Member

MarshallOfSound commented Mar 28, 2019

@ckerr @codebytere I'm going to unblock this, I've got a lot on my plate at the moment and it doesn't look like I'll be getting back to crash reporter specs for a bit. :shipit: at will

@codebytere codebytere merged commit 808783a into master Mar 28, 2019

6 checks passed

Semantic Pull Request ready to be squashed
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
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented Mar 28, 2019

No Release Notes

@codebytere codebytere deleted the clean-cr-params branch Mar 28, 2019

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