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

RESOLVED FIXED in Firefox 52

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: whimboo, Assigned: thomascharles94, Mentored)

Tracking

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

Version 3
mozilla53
good-first-bug, pi-marionette-server
Points:
---

Firefox Tracking Flags

(firefox52 fixed, firefox53 fixed)

Details

Attachments

(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
(Assignee)

Comment 1

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

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.

Thanks.
Flags: needinfo?(ato)
Assignee: nobody → thomascharles94
Status: NEW → ASSIGNED
Flags: needinfo?(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.
Attachment #8816753 - Flags: feedback?(ato) → feedback+
Comment hidden (mozreview-request)

Comment 5

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

https://reviewboard.mozilla.org/r/97492/#review97790
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:
https://hg.mozilla.org/integration/autoland/rev/250599a34188
Correct function definition style in Marionette components; r=ato

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/250599a34188
Status: ASSIGNED → RESOLVED
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
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/ee386af27b8b
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.