1310573 - Add reactjs/reselect to Devtools shared libraries

Status: RESOLVED FIXED

(DevTools :: Shared Components, defect, P1)

Description

[Jarda Snajdr [:jsnajdr]]

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.

Comment 1

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

We should also make sure that about:license page is properly updated.

Honza

Comment 2

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

Related newsgroup thread:
https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/okJlLHZ6V58

Honza

Comment 3

[Jarda Snajdr [:jsnajdr]]

Attachment: Bug 1310573 - Add reactjs/reselect to Devtools shared libraries

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

Comment 4

[Jarda Snajdr [:jsnajdr]]

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.

Comment 5

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

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

Comment 6

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8802113 [details]
Bug 1310573 - Add reactjs/reselect to Devtools shared libraries

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

Comment 7

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

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

Comment 8

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8802113 [details]
Bug 1310573 - Add reactjs/reselect to Devtools shared libraries

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

Comment 9

[Jarda Snajdr [:jsnajdr]]

(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.

Comment 10

[Gervase Markham [:gerv]]

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

[Dave Camp (:dcamp)]

a+

Comment 12

[Gervase Markham [:gerv]]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/709e4c09a28a
Add reactjs/reselect to Devtools shared libraries r=Honza

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/709e4c09a28a