1308480 - Move sorting functions to their own module

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

Netmonitor-view.js contains sorting functions like _byFile or _byTiming.

- Move them to a sort-predicates.js module. 
- Fix the sorting functions. Array.sort expects a function that returns -1/0/+1, but our sorting functions return true/false.

Comment 1

[Jarda Snajdr [:jsnajdr]]

QA instructions: verify that netmonitor columns are still sorted correctly.

Comment 2

[Jarda Snajdr [:jsnajdr]]

Attachment: Bug 1308480 - Move sorting functions to their own module

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

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8800195 [details]
Bug 1308480 - Move sorting functions to their own module

https://reviewboard.mozilla.org/r/85192/#review83760

Nice!


Just one nit, R+ assuming Try is green.
Honza

::: devtools/client/netmonitor/netmonitor-view.js:22
(Diff revision 1)
> -const {getFormDataSections, formDataURI, writeHeaderText, getKeyWithEvent} = require("./request-utils");
> +const {getFormDataSections, formDataURI, writeHeaderText,
> +       getKeyWithEvent, getUriHostPort} = require("./request-utils");

Put every imported function on separate line.

::: devtools/client/netmonitor/sort-predicates.js:7
(Diff revision 1)
> +
> +const { getAbbreviatedMimeType,
> +        getUriNameWithQuery,
> +        getUriHostPort,
> +        loadCauseString } = require("./request-utils");
> +

I like how the import statement is formatted, it would be great to  use it at other places too.
(see my next comment)

Comment 4

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8800195 [details]
Bug 1308480 - Move sorting functions to their own module

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b42244a8a8c
Move sorting functions to their own module r=Honza

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1b42244a8a8c

Comment 7

[Ciprian Georgiu, Desktop QA]

I can confirm that the netmonitor columns are still sorted correctly on latest Nightly build 52.0a1 (2016-10-17) under the following OSes:
- Windows 10 x64
- Mac OS X 10.11.4 
- Ubuntu 16.04 LTS

That being said I will mark this bug as verified fixed.