Skip to content

Conversation

@sebaslv
Copy link
Owner

@sebaslv sebaslv commented Nov 3, 2014

@busykai, please review this PR instead of #2. I have excluded dependencies from NodeSocketTrasnportDomain when integrating livedev2 files which makes that commit able to be reviewed now.

This is an initial review to prepare landing of livedev2 in Brackets core as per njx/brackets-livedev2#24

In preparation for landing livedev2 as an alternative implementation,
a folder /impls was created under LiveDevelopment folder and a the
current implementation moved to a /default folder.
Servers folder and LiveDevServerManager keep in their original location
since they will be used as shared resources for both implementations.
Make tests run after moving LiveDevelopment to /impls/default
Copy files from njx/brackets-livedev2.

Summary of changes to be integrated into brackets core:
- require context, module loading, paths were aligned to the new
  location
- preference names and main.js modified to work on top of current
  LiveDevelopment UI rather than adding a new launch icon
- dependencies for NodeSocketTransportDomain were not included
  (need to run npm install to get it working)

At this point, LiveDevelopment implementation can be manually
switched by changing the module that is being load at brackets.js:
/impls/default/main.js -> /impls/livedev2/main.js
Add livedev.impl preference to let the user switch between 'default'
and 'livedev2' implementations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I know it's not yours, but this should be aligned better.

@busykai
Copy link
Collaborator

busykai commented Nov 5, 2014

README.md for livedev2 should be part of this PR as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this outdated?

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes, it is working now, will remove TODO

@busykai
Copy link
Collaborator

busykai commented Nov 5, 2014

Why we need a copy of HTMLInstrumentation in livedev2? It could and should be shared.

It now shows the twipsy that informs the user when the session has
finished because of the browser/tab has been closed.
@sebaslv sebaslv mentioned this pull request Nov 18, 2014
@sebaslv
Copy link
Owner Author

sebaslv commented Nov 19, 2014

Closing in favor of #4. Feedback from this review was included but we conclude that this approach is not compatible with extensions that depend on LiveDevelopment.

@sebaslv sebaslv closed this Nov 19, 2014
sebaslv pushed a commit that referenced this pull request Apr 24, 2015
Review the Getting Started page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants