Add reactjs/reselect to Devtools shared libraries

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Shared Components
P1
normal
RESOLVED FIXED
2 years ago
7 months ago

People

(Reporter: jsnajdr, Assigned: jsnajdr)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 52
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [netmonitor])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
netmonitor.html will use https://github.com/reactjs/reselect. Add it to the devtools/client/shared/vendor directory.

Look at bug 1253784 (adding immutable.js) to see what it takes to add a 3rd party library - minified vs non-minified versions, who to ask for licence review etc.
(Assignee)

Updated

2 years ago
Blocks: 1307743
See Also: → bug 1253784

Updated

2 years ago
Flags: qe-verify?
Whiteboard: [netmonitor]

Updated

2 years ago
Priority: -- → P2

Updated

2 years ago
Flags: qe-verify? → qe-verify-
We should also make sure that about:license page is properly updated.

Honza
Comment hidden (mozreview-request)
(Assignee)

Comment 4

2 years ago
I followed these steps when adding the reselect.js file:

1. git clone https://github.com/reactjs/reselect - package.json says it's version 2.5.4.
2. npm install - compiles the JS to a final module file
3. cp dist/reselect.js $DEST_DIR - copies the resulting file to Firefox source tree

The library has a vanilla MIT license: https://github.com/reactjs/reselect/blob/master/LICENSE
Should I include a copy in the shared/vendor directory?

I'm using the reselect library already in my WIP work and I can confirm that it actually works.
Assignee: nobody → jsnajdr

Updated

2 years ago
Status: NEW → ASSIGNED
Iteration: --- → 52.3 - Nov 7
Priority: P2 → P1

Comment 5

2 years ago
mozreview-review
Comment on attachment 8802113 [details]
Bug 1310573 - Add reactjs/reselect to Devtools shared libraries

https://reviewboard.mozilla.org/r/86638/#review85568

Please create also a file with instructions how to upgrate the library. Something like you did in comment #4

The file name should be RESELECT_UPGRADING, to be consistent with existing files, see: REDUX_UPGRADING and REACT_UPGRADING as examples.

I believe that the license should be put into a file called: RESELECT_LICENSE, just like the existing e.g. REDUX_LICENSE, but I guess Gerv will know better.

Also, please make sure to update about:license page.

Honza
Attachment #8802113 - Flags: review?(odvarko)
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8802113 [details]
Bug 1310573 - Add reactjs/reselect to Devtools shared libraries

https://reviewboard.mozilla.org/r/86638/#review85874

One last thing, it looks like about:license page sorts all licenses alphabetically so, please put the 'Reselect License' entry after 'Redux License' entry.

R+ assuming my comment is resolved.

Thanks Jarda!

Honza
Attachment #8802113 - Flags: review?(odvarko) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 9

2 years ago
(In reply to Jan Honza Odvarko [:Honza] from comment #7)
> One last thing, it looks like about:license page sorts all licenses
> alphabetically so, please put the 'Reselect License' entry after 'Redux
> License' entry.

Damn, I thought I sorted it correctly the first time, but obviously I failed. I need to work more on my sorting skills. Fixed.
According to elvin, dcamp is currently thinking about React usage within Mozilla. Before I r= this I'd like him to comment.

Gerv

Comment 11

2 years ago
a+
Comment on attachment 8802113 [details]
Bug 1310573 - Add reactjs/reselect to Devtools shared libraries

https://reviewboard.mozilla.org/r/86638/#review88730

r=gerv.

Comment 13

2 years ago
Pushed by jsnajdr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/709e4c09a28a
Add reactjs/reselect to Devtools shared libraries r=Honza
Attachment #8802113 - Flags: review?(gerv) → review+

Comment 14

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

Updated

7 months ago
Blocks: 1408737
You need to log in before you can comment on or make changes to this bug.