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

Note for runtime.OnInstalledReason browser_update #300

Merged
merged 4 commits into from Sep 11, 2017

Conversation

Projects
None yet
4 participants
@nkestrel

nkestrel commented Jul 27, 2017

As per Bug 1252871 comment 45 and the final patch.

Can we just call this "browser_update"

Sgtm.

@wbamberg

Thanks for this! But I think it would be better to treat browser_update as the "normal" case, and chrome_update as the deviation.

That is, add the note to Chrome and Opera, and reverse the text:

"Uses 'chrome_update' instead of 'browser_update'."

The reasons for this are:

  • the docs refer to browser_update
  • browser_update is a more generic term (we'd hope that other browsers would use it in preference to chrome_update)
@nkestrel

This comment has been minimized.

Show comment
Hide comment
@nkestrel

nkestrel Jul 28, 2017

I wasn't sure about other browsers, does Edge not support chrome_update? I looked up the Edge docs but they redirect to MDN.

nkestrel commented Jul 28, 2017

I wasn't sure about other browsers, does Edge not support chrome_update? I looked up the Edge docs but they redirect to MDN.

@wbamberg

This comment has been minimized.

Show comment
Hide comment
@wbamberg

wbamberg Aug 2, 2017

Member

I don't know about Edge. @erikadoyle , do you know if Edge uses chrome_update or browser_update here?

Member

wbamberg commented Aug 2, 2017

I don't know about Edge. @erikadoyle , do you know if Edge uses chrome_update or browser_update here?

@erikadoyle

This comment has been minimized.

Show comment
Hide comment
@erikadoyle

erikadoyle Aug 7, 2017

Contributor

Edge currently supports only the install and update values, but fwiw we prefer browser_update. Thanks!

Contributor

erikadoyle commented Aug 7, 2017

Edge currently supports only the install and update values, but fwiw we prefer browser_update. Thanks!

@wbamberg

Thanks @nkestrel and @erikadoyle !

@wbamberg wbamberg merged commit 1d304ed into mdn:master Sep 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment