1316975 - Update marionette server .js modules for function definition style (space before opening bracket)

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Right now the .js modules for Marionette server define methods like that:

> foo = function(arg) {}

This can cause confusion because it could also mean that a function with the name "function" gets called here. To avoid that and to be conform with the Mozilla JS style guide we should change all those instances to:

> foo = function (arg) {}

Andreas, do you want to mentor this bug? Looks like a good-first-bug one.

Comment 1

[Thomas Charles]

Attachment: Bug1316975.patch

Comment 2

[Thomas Charles]

Hi Andreas,

I'm still relatively new to fixing bugs. I have uploaded a patch of the changes I think are needed.

Could you let me know if this is the correct approach to this bug and if any further changes need to made?

If the patch is looking good, can I have this bug assigned to me.

Thanks.

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8816753 [details] [diff] [review]
Bug1316975.patch

Review of attachment 8816753 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good!  Thanks.

I’m going to submit this to MozReview and run some relevant try jobs, and if they pass, submit it for integration.

For your next patch, you might want to consider submitting it to MozReview yourself.

Comment 4

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1316975 - Correct function definition style in Marionette components;

No functional changes.

Review commit: https://reviewboard.mozilla.org/r/97492/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/97492/

Comment 5

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8817057 [details]
Bug 1316975 - Correct function definition style in Marionette components;

https://reviewboard.mozilla.org/r/97492/#review97790

Comment 6

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8817057 [details]
Bug 1316975 - Correct function definition style in Marionette components;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97492/diff/1-2/

Comment 7

[Andreas Tolfsen ❲:ato❳]

I have requested integration of this now.  Sorry for taking so long.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/250599a34188
Correct function definition style in Marionette components; r=ato

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/250599a34188

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I propose we also get this test-only patch landed on aurora to lessen the work for upcoming uplifts of the 52esr releases.

Comment 11

[Carsten Book [:Tomcat]]

needs rebasing 

grafting 379302:250599a34188 "Bug 1316975 - Correct function definition style in Marionette components; r=ato"
merging testing/marionette/addon.js
merging testing/marionette/components/marionettecomponent.js and testing/marionette/components/marionette.js to testing/marionette/components/marionettecomponent.js
merging testing/marionette/driver.js
merging testing/marionette/error.js
merging testing/marionette/listener.js
merging testing/marionette/navigate.js
warning: conflicts while merging testing/marionette/addon.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/marionette/driver.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It might be not worth the time then. I would leave the decision up to Andreas.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: uplift.patch

Given that other patches have been uplifted the number of merge conflicts for this patch is only 1. So let go and uplift it to 52, because it would cause lots of conflicts for other upcoming patches we want to uplift.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Please uplift the test-only patch as updated in attachment 8823230 [details] [diff] [review] to aurora. Thanks.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ee386af27b8b