1452061 - CamelCase all React component files in /devtools/client/performance/components/

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

We recently agreed to name our React component file names using the CamelCase convention.

However the files in /devtools/client/performance/components/ do not follow this convention, so let's rename them.

This is an easy fix, but make sure you read the contribution docs before starting: http://docs.firefox-dev.tools/ so that you can build Firefox locally and verify that things still work after your change.
After building, just running the built Firefox and opening the memory panel (by first enabling it from the settings panel) should be enough of a test that things still work.

Comment 1

[Julian Descottes [:jdescottes]]

Sorry for copy pasting a summary, I assumed the last part was just a set of generic instructions:

"After building, just running the built Firefox and opening the memory panel (by first enabling it from the settings panel) should be enough of a test that things still work."

Should be 

"After building, just running the built Firefox and opening the Performance panel (by first enabling it from the settings panel) should be enough of a test that things still work"

Comment 2

[Javed Nissar]

Just to let everyone know, I'm almost done with a patch for this issue. I'm just making some finishing touches.

Comment 3

[Javed Nissar]

Attachment: Bug 1452061 - renamed React component files in devtools/client/performance to be CamelCase.

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

Comment 4

[Greg Tatum [:gregtatum]]

Comment on attachment 8968485 [details]
Bug 1452061 - renamed React component files in devtools/client/performance to be CamelCase.

https://reviewboard.mozilla.org/r/237186/#review243150

Hey thanks for taking a stab at this. To clarify a bit, the intent was on `CamelCase` for the components, and not `camelCase`. Also, this review is confusing to look at, since it appears that all the files are deleted and then recreated. Mercurial has a nice built in command to let you rename files so that it understands when things have moved, and not been deleted: http://hgbook.red-bean.com/read/mercurial-in-daily-use.html#id364064 This would make the review a lot easier to look at. Thanks!

Comment 5

[Whitney Meulink]

Hi there, I am a student at a University working with a team of 3 other students who are also gaining experience in Open Source. We thought that this bug would be a good first bug to get familiar with the Patch process. 

We are wondering if you could clarify what the React component files are in /devtools/client/performance/components/. Just want to make sure that to fix this, we simply need to rename the files, such as recording-button.js to RecordingButton.js.

Any information you can give is much appreciated. Thank you!

Comment 6

[Javed Nissar]

Comment on attachment 8968485 [details]
Bug 1452061 - renamed React component files in devtools/client/performance to be CamelCase.

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

Comment 7

[Javed Nissar]

Hey, I have another draft up. Unfortunately, though I wasn't able to replicate what you mentioned about renaming files. I tried using hg mv as stated in your link but it still displays the diff as a removal + addition.

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8968485 [details]
Bug 1452061 - renamed React component files in devtools/client/performance to be CamelCase.

https://reviewboard.mozilla.org/r/237186/#review256286

Thanks for the patch but as Greg mentioned previously, we need to use `hg move` in order to preserve the source history and make the diff more readable.

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1452061 - CamelCase all React component files in devtools performance panel;

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

Comment 10

[Julian Descottes [:jdescottes]]

green try push for last patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af93d36aef3e58098a216d3cef734913a99ed8fb

Comment 11

[Greg Tatum [:gregtatum]]

Comment on attachment 8985521 [details]
Bug 1452061 - CamelCase all React component files in devtools performance panel;

https://reviewboard.mozilla.org/r/251134/#review262498

Sorry the delay on the view, it all looks reasonable to me! Thanks!

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8985521 [details]
Bug 1452061 - CamelCase all React component files in devtools performance panel;

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9162b1c01237
CamelCase all React component files in devtools performance panel;r=gregtatum

Comment 14

[Raul Gurzau (:RaulG)]

Backed out changeset 9162b1c01237 (bug 1452061) for failing at performance/components/test/test_jit_optimizations_01.html on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/e679e55fc5d485d52ce5893bb2cb5315a1839390

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9162b1c012371b3bb2d2f941e9d54f38fc33f73e

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=189783426&repo=autoland&lineNumber=5107

Log snippet: 

[task 2018-07-24T13:25:16.232Z] 13:25:16     INFO - SimpleTest START
[task 2018-07-24T13:25:16.248Z] 13:25:16     INFO - TEST-START | devtools/client/performance/components/test/test_jit_optimizations_01.html
[task 2018-07-24T13:25:16.332Z] 13:25:16     INFO - GECKO(3179) | ++DOMWINDOW == 22 (0xe3270400) [pid = 3179] [serial = 22] [outer = 0xe2df8d90]
[task 2018-07-24T13:25:17.302Z] 13:25:17     INFO - GECKO(3179) | ++DOMWINDOW == 23 (0xe2df4400) [pid = 3179] [serial = 23] [outer = 0xe2df8d90]
[task 2018-07-24T13:25:17.784Z] 13:25:17     INFO - TEST-INFO | started process screentopng
[task 2018-07-24T13:25:18.352Z] 13:25:18     INFO - TEST-INFO | screentopng: exit 0
[task 2018-07-24T13:25:18.353Z] 13:25:18     INFO - TEST-UNEXPECTED-FAIL | devtools/client/performance/components/test/test_jit_optimizations_01.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once.  Make sure you use SimpleTest.waitForExplicitFinish() if you need it.) 
[task 2018-07-24T13:25:18.354Z] 13:25:18     INFO - afterCleanup@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1187:13
[task 2018-07-24T13:25:18.355Z] 13:25:18     INFO - executeCleanupFunction@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1221:13
[task 2018-07-24T13:25:18.357Z] 13:25:18     INFO - SimpleTest.finish@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1240:5
[task 2018-07-24T13:25:18.359Z] 13:25:18     INFO - window.onload@chrome://mochitests/content/chrome/devtools/client/performance/components/test/test_jit_optimizations_01.html:64:5
[task 2018-07-24T13:25:18.360Z] 13:25:18     INFO - EventHandlerNonNull*@chrome://mochitests/content/chrome/devtools/client/performance/components/test/test_jit_optimizations_01.html:16:1

Comment 15

[Julian Descottes [:jdescottes]]

Comment on attachment 8985521 [details]
Bug 1452061 - CamelCase all React component files in devtools performance panel;

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

Comment 16

[Julian Descottes [:jdescottes]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=76c09d2334b5f2e503c1e7723893ea853eefdb0b

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cc1be9e8d14
CamelCase all React component files in devtools performance panel;r=gregtatum

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5cc1be9e8d14