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

Make MenuItem roles camelCase-compatible #11532

Merged
merged 3 commits into from Jan 4, 2018

Conversation

Projects
None yet
3 participants
@sethlu
Member

sethlu commented Dec 28, 2017

Migrating MenuItem roles to be in camelCase while keeping backward lowercase compatibility.

Closes #10372
CC: @zeke

@sethlu sethlu requested review from electron/docs as code owners Dec 28, 2017

@codebytere

This comment has been minimized.

Member

codebytere commented Dec 30, 2017

hey @sethlu, you'll need to fix linting errors here before we can begin to review this. thanks!

@sethlu

This comment has been minimized.

Member

sethlu commented Dec 31, 2017

@codebytere cool! I’ll do that a little later today 👍

@codebytere

This comment has been minimized.

Member

codebytere commented Jan 3, 2018

hey @sethlu, it looks like you're failing 342 Menu module MenuItem editMenu includes a default submenu layout when submenu is empty; what's happening locally?

Revert changes made to test case
The MenuItem role should be lowercase
@sethlu

This comment has been minimized.

Member

sethlu commented Jan 4, 2018

@codebytere I forgot to run the test cases again... 😢 Should work now.

@codebytere

This looks good to me! Appreciate the standardization :)

@zeke zeke merged commit eb89e12 into electron:master Jan 4, 2018

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
@zeke

This comment has been minimized.

Member

zeke commented Jan 4, 2018

Nice! Thanks @sethlu.

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