feat: add window removeMenu() method

[codebytere]

Description of Change

This PR adds win.removeMenu() as a replacement for win.setMenu(null), which is less intuitive and a bit of an antipattern for modern JS. We can most likely backport the additional method into 5-0-x and only remove the original in master.

cc @MarshallOfSound @ckerr

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• relevant documentation is changed or added
• PR title follows semantic commit guidelines
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Added win.removeMenu() to remove application menus instead of using win.setMenu(null).

[miniak]

@codebytere I don't like this change much. In many frameworks, you would have a property menu or something similar, where assigning null to that would remove the menu, icon, etc.

[codebytere]

@miniak JS frameworks, & modern ones? Using null to mean no argument as a removal mechanism is an anti-pattern at this point, as new features like default parameters only work with undefined

I think explicit functions for removal should be preferred over some of our current patterns, with the end goal being getters and setters

[miniak]

@codebytere how would the API look like with getters/setters? you won't be able to assign null / undefined to the property and we'll still have the remove method? the getter can still return a falsy value. Which means that win.menu = win.menu can blow up :)

[MarshallOfSound]

@miniak This is an API improvement we can do in a non-breaking way to better support modern styles of JS. Calling setThing(null) to remove it is not as idiomatic as removeThing().

This isn't a final "this is the best API we can build", this is a "this is the best API we can build without changing our entire API". Discussions around future API design like setters and getters instead of methods are a much wider discussion and something that I believe is on the list of proposed topics for summit.

For now, this PR is a noticeable API improvement and reduces confusion around usage.

[codebytere]

2019-01-31T02:57:43.7306876Z atom/browser/atom_download_manager_delegate.cc:130:  You don't need a ; after a }

🤔 i don't even change that file in this PR

[release-clerk]

Release Notes Persisted

Added win.removeMenu() to remove application menus instead of using win.setMenu(null).

[trop]

I have automatically backported this PR to "5-0-x", please check out #16657