Move sorting functions to their own module

VERIFIED FIXED in Firefox 52

Status

P1
normal
VERIFIED FIXED
2 years ago
4 months ago

People

(Reporter: rickychien, Assigned: jsnajdr)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 52
Dependency tree / graph

Firefox Tracking Flags

(firefox52 verified)

Details

(Whiteboard: [netmonitor])

Attachments

(1 attachment)

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.

Updated

2 years ago
Whiteboard: [devtools-html]
(Reporter)

Updated

2 years ago
Depends on: 1308690
(Reporter)

Updated

2 years ago
Blocks: 1308690
No longer depends on: 1308690

Updated

2 years ago
Whiteboard: [netmonitor]

Updated

2 years ago
Flags: qe-verify+
QA Contact: ciprian.georgiu
(Assignee)

Comment 1

2 years ago
QA instructions: verify that netmonitor columns are still sorted correctly.
Assignee: nobody → jsnajdr
Depends on: 1309120
Comment hidden (mozreview-request)

Updated

2 years ago
Status: NEW → ASSIGNED

Comment 3

2 years ago
mozreview-review
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)
Attachment #8800195 - Flags: review?(odvarko) → review+
Comment hidden (mozreview-request)

Comment 5

2 years ago
Pushed by jsnajdr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1b42244a8a8c
Move sorting functions to their own module r=Honza

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1b42244a8a8c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52

Updated

2 years ago
Iteration: --- → 52.2 - Oct 17

Updated

2 years ago
Priority: -- → P1
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.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
Flags: qe-verify+

Updated

4 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.