1302062 - Switch recording list to React component for perf tool

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect, P2)

Comment 1

[Greg Tatum [:gregtatum]]

Attachment: Bug 1302062 - Use React on performance recording list;

I split out the test changes into separate commits to make the review
easier. I plan on folding them all into one commit when I push them
so that the tests will pass on all commits.

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

Comment 2

[Greg Tatum [:gregtatum]]

Attachment: Bug 1302062 - Remove RecordingsView from tests;

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

Comment 3

[Greg Tatum [:gregtatum]]

Attachment: Bug 1302062 - Rewrite browser_perf-console-record-08.js to be easier to understand;

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

Comment 4

[Greg Tatum [:gregtatum]]

Comment on attachment 8792906 [details]
Bug 1302062 - Use React on performance recording list;

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

Comment 5

[Greg Tatum [:gregtatum]]

Comment on attachment 8792907 [details]
Bug 1302062 - Remove RecordingsView from tests;

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

Comment 6

[Greg Tatum [:gregtatum]]

Comment on attachment 8792908 [details]
Bug 1302062 - Rewrite browser_perf-console-record-08.js to be easier to understand;

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

Comment 7

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b89749a157b

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8792906 [details]
Bug 1302062 - Use React on performance recording list;

https://reviewboard.mozilla.org/r/79770/#review78798

Looks good! Great comments in the RecordingsView too, w/r/t migration.

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8792907 [details]
Bug 1302062 - Remove RecordingsView from tests;

https://reviewboard.mozilla.org/r/79772/#review78800

This looks like it was a fun one, heh. LGTM

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8792908 [details]
Bug 1302062 - Rewrite browser_perf-console-record-08.js to be easier to understand;

https://reviewboard.mozilla.org/r/79774/#review78802

LGTM!!

::: devtools/client/performance/test/browser_perf-console-record-08.js:30
(Diff revision 2)
> +// Represents a recorded profile from console.profile().
> +const CONSOLE = 1;
> +// Represents a profile that is currently recording.
> +const RECORDING = 2;
> +// Represents a profile that is currently selected.
> +const SELECTED = 4;

Nice, much cleaner than before

Comment 11

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98ca54094b1a

Comment 12

[Greg Tatum [:gregtatum]]

Comment on attachment 8792906 [details]
Bug 1302062 - Use React on performance recording list;

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c651f7174f37
Use React on performance recording list; r=jsantell

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c651f7174f37