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

Zaggino/update extension manager deps #12933

Merged
merged 8 commits into from
Mar 15, 2017

Conversation

zaggino
Copy link
Contributor

@zaggino zaggino commented Nov 22, 2016

Replacement for #10372 to keep from commiting node_modules to git as we should really abandon that practice where possible.

Related #12898

cc @ficristo

Note: the webpack approach for semver is taken from brackets-electron, where it's implemented and works well if dependencies (in this case semver) don't provide amd build that we need to use in the browser code

Note2: sorry the diff is quite hard to read because of all the deletes, reading through commits might be easier

@ficristo
Copy link
Collaborator

I would prefer if someone more familiar with webpack could take a look.
Maybe someone at Adobe?

@zaggino
Copy link
Contributor Author

zaggino commented Dec 12, 2016

@ficristo this is now up to date with master after #12972 merge. Not sure if anyone from Adobe will have time to look at this anytime soon.

@ficristo
Copy link
Collaborator

I'm sorry, but I am really not confident in reviewing this.
Maybe @petetnt or @MiguelCastillo can?

@ficristo ficristo requested a review from swmitra December 19, 2016 20:33
@ficristo ficristo added this to the Release 1.9 milestone Dec 20, 2016
@ficristo ficristo requested a review from madanbn January 6, 2017 07:59
@ficristo
Copy link
Collaborator

ficristo commented Jan 9, 2017

src/extensibility/node is exported as a zip for the registry (see brackets-registry/deps), so it seems to me it needs to be a standalone project or at least we need some way to export the zip: from what I can tell this breaks it.
@zaggino do you know how the zip is created? Or maybe @ingorichter or @redmunds.

@redmunds
Copy link
Contributor

I believe that src/extensibility is used by both Brackets and the Brackets Extension Registry, but I don't know the process from getting code onto the registry. @ingorichter Do you know?

@ingorichter
Copy link
Contributor

we need to tar/gzip all the js files in brackets/src/extensibility/node/ to create the archive for the registry. Then someone with permissions needs to update the registry. I don't have permissions anymore to the server. Before that update, we need to make sure that the changes work locally before updating the live system. I once ran into issues on that system, since the node version might be pretty outdated again and therefore required node_modules might not like it.

@zaggino
Copy link
Contributor Author

zaggino commented Feb 15, 2017

@swmitra can someone at @adobe take care of the update for https://github.com/adobe/brackets-registry or can I get access to that machine and work on it? we need to work on getting this merged so we can fix bugs related to brackets not working properly behind a proxy server

@zaggino zaggino force-pushed the zaggino/update-extension-manager-deps branch from c264a33 to cc62ef8 Compare February 15, 2017 04:22
@zaggino
Copy link
Contributor Author

zaggino commented Feb 15, 2017

updated this PR to be merge-able

@swmitra
Copy link
Collaborator

swmitra commented Feb 16, 2017

@zaggino We are looking into the registry server node upgrade request. Should be able to give you some concrete data by end of this week.

@saurabh95
Copy link
Contributor

@zaggino Since the server has upgraded node version, Could you resolve conflicts in this PR?

@zaggino
Copy link
Contributor Author

zaggino commented Mar 15, 2017

@saurabh95 Conflicts resolved.

@swmitra
Copy link
Collaborator

swmitra commented Mar 15, 2017

@zaggino Not yet 😄 . Probably becuase I have merged @ficristo PR for write config grunt task. Can you please merge once more.

@zaggino
Copy link
Contributor Author

zaggino commented Mar 15, 2017

@swmitra Done again :-)

@swmitra
Copy link
Collaborator

swmitra commented Mar 15, 2017

Merging this PR. Build failure is because of cla check pull failure.

@swmitra swmitra merged commit 1882434 into master Mar 15, 2017
@swmitra swmitra deleted the zaggino/update-extension-manager-deps branch March 15, 2017 07:48
@swmitra
Copy link
Collaborator

swmitra commented Mar 15, 2017

Thank a ton @zaggino for bringing this change. It makes dependency management a lot more trivial. Great work as always 👍

Copy link
Collaborator

@swmitra swmitra left a comment

Choose a reason for hiding this comment

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

Good to merge as registry server is also upgraded to latest node.

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

Successfully merging this pull request may close these issues.

6 participants