Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Bringing in ImmutableJS 3.8.2 #14168

Merged

Conversation

vickramdhawal
Copy link
Collaborator

@vickramdhawal vickramdhawal commented Mar 22, 2018

Bringing in ImmutableJS 3.8.2 which is release under MIT license. Source of immutableJS https://github.com/facebook/immutable-js/releases/tag/v3.8.2.
Also updating the gitignore file to exclude node_modules inside src/JSUtils

@nethip
Copy link
Contributor

nethip commented Mar 22, 2018

Thanks for the PR @vickramdhawal 👍

Did you get a chance to run unit tests?

@nethip
Copy link
Contributor

nethip commented Mar 22, 2018

Also looks like CLA has not been signed that is why the travis tests are failing. Can you signup using this link? Please sign the CLA here http://dev.brackets.io/brackets-contributor-license-agreement.html.

@vickramdhawal
Copy link
Collaborator Author

vickramdhawal commented Mar 22, 2018

I've signed up now. I did run all tests, but have to bear with multiple dialogs to get it to completion. Will update once its done.

@nethip
Copy link
Contributor

nethip commented Mar 22, 2018

Thanks @vickramdhawal I have restarted the build.

@nethip
Copy link
Contributor

nethip commented Mar 22, 2018

You can disable the dialog popups by returning from the below function. We will have to figure out other means of handling the dialog popups.
https://github.com/adobe/brackets/blob/master/src/document/DocumentCommandHandlers.js#L1727

@vickramdhawal
Copy link
Collaborator Author

immutableJS is used only by the FileTreeView & the FileTreeViewModel and the tests are passing for both. Unit tests are also passing except for the ones already failing on master.

@swmitra
Copy link
Collaborator

swmitra commented Mar 23, 2018

Good work @vickramdhawal.

@swmitra swmitra merged commit 43cfabb into adobe:master Mar 23, 2018
@@ -12,6 +12,8 @@ package-lock.json

# ignore node_modules inside src
/src/node_modules
/src/JSUtils/node_modules
/src/JSUtils/node/node_modules
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this fixes the effects and not the cause of the problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants