Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-13431] LiveView: iOS Simulator LiveView fails to build #12

Merged
merged 4 commits into from Apr 13, 2013
Merged

[TIMOB-13431] LiveView: iOS Simulator LiveView fails to build #12

merged 4 commits into from Apr 13, 2013

Conversation

euforic
Copy link
Contributor

@euforic euforic commented Apr 12, 2013

[TIMOB-13431] LiveView: iOS Simulator LiveView fails to build

@@ -9,7 +9,30 @@ var http = require('http')
, fs = require('fs')
, fsWatcher = require('chokidar')
, path = require('path')
, colors = require('colors');
Copy link
Contributor

Choose a reason for hiding this comment

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

Comma first is not allowed! It violates our style guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

spoke with ingo he said it's fine for now in liveview we will revisit in planning week.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, that's not what I meant. I mean, it should follow the JS guidelines now. If we want to change it as a group, we can do so in a discussion during planning week.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ingo ok I will fix 👎

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh pls don't change style convention to comma first!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jhaynie I only like it for module definitions

@nebrius
Copy link
Contributor

nebrius commented Apr 13, 2013

In testing I get this:
[DEBUG] Bad plugin hooks that failed to load:
[DEBUG] /Users/bhughes/Work/liveview/hook/lvhook.js

@cb1kenobi
Copy link
Contributor

Bad plugin hook error happens when the hook has a js syntax error.

@euforic
Copy link
Contributor Author

euforic commented Apr 13, 2013

@bryan-m-hughes how are you installing ? $ npm link

@euforic
Copy link
Contributor Author

euforic commented Apr 13, 2013

@bryan-m-hughes the hook path is wrong it should be /usr/local/lib/node_modules/liveview/bin/../hook

@nebrius
Copy link
Contributor

nebrius commented Apr 13, 2013

npm link/install should never be required to run a node.js project. It should always be possible to run directly. This absolutely needs to be fixed for 3.1.1.

@euforic
Copy link
Contributor Author

euforic commented Apr 13, 2013

@bryan-m-hughes install/link is not required they just add and remove the hook for you if they are called. At minimum you will have to run npm install with no args from the modules root directory to install its dependencies.

@nebrius
Copy link
Contributor

nebrius commented Apr 13, 2013

@euforic in the future please say so. Also, my path was not wrong.

@nebrius
Copy link
Contributor

nebrius commented Apr 13, 2013

Code reviewed and tested. Request accepted.

nebrius added a commit that referenced this pull request Apr 13, 2013
[TIMOB-13431] LiveView: iOS Simulator LiveView fails to build
@nebrius nebrius merged commit 1e6634e into tidev:master Apr 13, 2013
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.

None yet

5 participants