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: use helpers for command-line parsing in renderer/init.js #16239

Merged
merged 1 commit into from Jan 3, 2019

Conversation

Projects
None yet
4 participants
@miniak
Copy link
Contributor

miniak commented Jan 2, 2019

Description of Change

Makes the command-line parsing more robust.

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 electron/reviewers as a code owner Jan 2, 2019

@miniak miniak force-pushed the miniak/init-commandline branch 2 times, most recently from 11b7eb9 to 074527a Jan 2, 2019

Show resolved Hide resolved lib/renderer/command-line.js Outdated
@nornagon
Copy link
Contributor

nornagon left a comment

i like this change, i think it would be even better if we used CommandLine::HasSwitch directly!

@miniak miniak force-pushed the miniak/init-commandline branch from 074527a to 945f776 Jan 3, 2019

@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Jan 3, 2019

@nornagon I've exposed hasSwitch and getSwitchValue wrapping base::CommandLine from native code

@deepak1556
Copy link
Member

deepak1556 left a comment

Failing test seems related to this PR.

Show resolved Hide resolved atom/browser/web_contents_preferences.cc Outdated
Show resolved Hide resolved atom/common/api/atom_api_command_line.cc Outdated

@miniak miniak force-pushed the miniak/init-commandline branch from 945f776 to 7dd3305 Jan 3, 2019

@miniak miniak force-pushed the miniak/init-commandline branch from 7dd3305 to 55f9846 Jan 3, 2019

@miniak miniak referenced this pull request Jan 3, 2019

Merged

chore: disable nodeIntegration / webviewTag by default #16235

6 of 6 tasks complete
@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Jan 3, 2019

@deepak1556 all comments should be addressed

@jkleinsc

This comment has been minimized.

Copy link
Contributor

jkleinsc commented Jan 3, 2019

Merging with failing arm test as it is broken in master

@jkleinsc jkleinsc merged commit 3f1d227 into master Jan 3, 2019

21 checks passed

Absolute Zero
Semantic Pull Request ready to be squashed
Details
WIP Legacy commit status override — see details
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
ci/circleci: linux-arm-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-arm-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-arm64-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-arm64-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-checkout Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-testing-tests Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-testing-tests Your tests passed on CircleCI!
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented Jan 3, 2019

No Release Notes

@codebytere codebytere deleted the miniak/init-commandline branch Jan 4, 2019

andrewkisliakov-citrix added a commit to andrewkisliakov-citrix/electron that referenced this pull request Jan 22, 2019

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