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

Move CodeMirror on node_modules#12963

Closed
ficristo wants to merge 2 commits into
adobe:masterfrom
ficristo:npm-codemirror
Closed

Move CodeMirror on node_modules#12963
ficristo wants to merge 2 commits into
adobe:masterfrom
ficristo:npm-codemirror

Conversation

@ficristo

@ficristo ficristo commented Dec 2, 2016

Copy link
Copy Markdown
Collaborator

What I had in mind was something like this.
I think this will NOT work on distribution since the node_modules would be in a different location.
Also I find worrisome I had to change the css link.

Ref #12940

@zaggino

zaggino commented Dec 2, 2016

Copy link
Copy Markdown
Contributor

Looks good to me, let's test how the distribution mode works.

@ficristo

ficristo commented Dec 3, 2016

Copy link
Copy Markdown
Collaborator Author

Otherwise we could put another package.json in the src folder.
But the two package.json doesn't seem necessary anymore with the newer versions of electron-builder and I think this is a bit of a contributor barrier.
I still don't have any idea how to change the path for distribution in case of single package.json.
At this point I don't have strong opinions: thoughts?

@swmitra

swmitra commented Dec 6, 2016

Copy link
Copy Markdown
Collaborator

@ficristo The changes looks good to me. But I have a different question. When I tried to check the changes on my local machine, I had to do a npm install. Is that expected when someone clones the repo for dev setup?

@zaggino

zaggino commented Dec 6, 2016

Copy link
Copy Markdown
Contributor

@swmitra yes, npm install is expected, it has also been added here: https://github.com/adobe/brackets/wiki/How-to-Hack-on-Brackets#setting-up-your-dev-environment

@ficristo

ficristo commented Dec 6, 2016

Copy link
Copy Markdown
Collaborator Author

I tryed to hack something but I wasn't able to make grunt build to pass. (In the specific grunt requirejs)
If nobody has some bright idea I don't think this will work, as is, on distribution.
I'll create a new package.json inside src and see how this will work.
Of course #12940 is still a choice.

@ficristo

ficristo commented Dec 8, 2016

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #12972.

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.

3 participants