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

RESOLVED FIXED in Firefox 52


2 years ago
2 years ago


(Reporter: whimboo, Assigned: thomascharles94, Mentored)


({good-first-bug, pi-marionette-server})

Version 3
good-first-bug, pi-marionette-server

Firefox Tracking Flags

(firefox52 fixed, firefox53 fixed)



(3 attachments)

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.
Flags: needinfo?(ato)
Flags: needinfo?(ato)
Mentor: ato
Keywords: ateam-marionette-server, good-first-bug

Comment 1

2 years ago
Attachment #8816753 - Flags: feedback?(ato)

Comment 2

2 years ago
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.

Flags: needinfo?(ato)
Assignee: nobody → thomascharles94
Flags: needinfo?(ato)
Comment on attachment 8816753 [details] [diff] [review]

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.
Attachment #8816753 - Flags: feedback?(ato) → feedback+
Comment hidden (mozreview-request)

Comment 5

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

Attachment #8817057 - Flags: review?(ato) → review+
Comment hidden (mozreview-request)
I have requested integration of this now.  Sorry for taking so long.

Comment 8

2 years ago
Pushed by atolfsen@mozilla.com:
Correct function definition style in Marionette components; r=ato

Comment 9

2 years ago
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
I propose we also get this test-only patch landed on aurora to lessen the work for upcoming uplifts of the 52esr releases.
status-firefox52: --- → affected
Whiteboard: [checkin-needed-aurora]
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')
Flags: needinfo?(hskupin)
Whiteboard: [checkin-needed-aurora]
It might be not worth the time then. I would leave the decision up to Andreas.
status-firefox52: affected → fix-optional
Flags: needinfo?(hskupin)
Posted patch uplift.patchSplinter Review
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.
Please uplift the test-only patch as updated in attachment 8823230 [details] [diff] [review] to aurora. Thanks.
Whiteboard: [checkin-needed-aurora]

Comment 15

2 years ago
status-firefox52: fix-optional → fixed
Whiteboard: [checkin-needed-aurora]
You need to log in before you can comment on or make changes to this bug.