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

Update readme for electron app #105

Merged
merged 2 commits into from
Dec 21, 2015
Merged

Conversation

mattfelsen
Copy link
Member

First pass at adding some documentation on how to get the electron fronted up & running. Some of the docs will probably soon be outdated if folders are renamed (which I do in my forthcoming PR) and there are some steps which should get automated or have scripts for, but I believe this is what's necessary as-is, so we gotta start somewhere 😄

@mattfelsen
Copy link
Member Author

Helpful for issues like #104 and #83 (comment)

@mattfelsen
Copy link
Member Author

Pinging @danoli3 too who dug into this recently, would be good to know if this is a helpful format

@danoli3
Copy link
Member

danoli3 commented Dec 21, 2015

Awesome thanks! Makes a lot more sense :D!

On 21 December 2015 at 10:30, Matt Felsen notifications@github.com wrote:

Pinging @danoli3 https://github.com/danoli3 too who dug into this
recently, would be good to know if this is a helpful format


Reply to this email directly or view it on GitHub
#105 (comment)
.

@ofZach
Copy link
Contributor

ofZach commented Dec 21, 2015

this looks great - thanks! merging!

ofZach added a commit that referenced this pull request Dec 21, 2015
Update readme for electron app
@ofZach ofZach merged commit 9efe69e into openframeworks:master Dec 21, 2015
@mattfelsen mattfelsen deleted the docs branch December 21, 2015 17:57
@Daandelange
Copy link
Member

Thanks, nice start!
Maybe we could also put a link to npm which is not installed by default.

@mattfelsen
Copy link
Member Author

Oh good point, I always forget that about node/npm. I'll add a note and
suggest a couple ways to install:

  • easiest: from nodejs.org
  • homebrew
  • homebrew + nvm

Is anyone aware of any node version dependencies? I've used & built the
electron app with 0.10 and 5.3 so I think we're in the clear, but just
checking

On Mon, Dec 21, 2015 at 5:26 PM Daan de Lange notifications@github.com
wrote:

Thanks, nice start!
Maybe we could also put a link to npm https://nodejs.org/en/download/
which is not installed by default.


Reply to this email directly or view it on GitHub
#105 (comment)
.

@mattfelsen
Copy link
Member Author

Whoops, that's homebrew + nvm. Corrected

@ofZach
Copy link
Contributor

ofZach commented Dec 21, 2015

one thing I'd like to mention in the readme (I am making an issue here just to remind myself to add a note) is the use of "npm-shrinkwrap.json" ie:

https://github.com/openframeworks/projectGenerator/blob/master/projectGeneratorElectron/npm-shrinkwrap.json

to fix a bug that was building packages on osx incorrectly so they were failing on 10.8. I think it's useful to mention this in case anyone starts manipulating the modules / etc as I'm not sure if shrinkwrap needs to be regenerated, etc.

(also this bug might be fixed by now so if we can remove this, it make the whole thing less fragile)

I need to do a bit of research to remember the problem and solution but I'll add this shortly.

@mattfelsen
Copy link
Member Author

@ofZach I played around with shrinkwrap for a bit but could not get it to work! npm install works and the app starts fine, but then trying npm shrinkwrap from my working setup kept giving errors, mostly around a version mismatch between electron-debug 0.1.1 being wanted but electron-debug 0.2.1 being installed? Also it looks like devDependencies are ignored for shrinkwrapping, so those might need to be moved to dependencies (I tried this, and shrinkwrap is still complaining).

So I can't really test this, and I'm not sure how to test/reproduce the issues you saw on 10.8, so I'll leave it to you to figure out 😄 Here's the bit I wrote for the readme which I'm leaving out for now, but feel free to re-use later if you like:

Developing

This package is shrinkwrapped!

We use npm shrinkwrap to specify versions not only of the node modules we use directly, but their dependencies as well. If you are working on the electron app and need to add npm modules, be sure to update the shrinkwrap after and include npm-shrinkwrap.json in your pull request along with package.json and any code updates.

# the usual
npm install --save new-module

# do some coding, make sure everything works

# shrinkwrap it!
npm shrinkwrap

# add npm-shrinkwrap.json when you commit!

@ofZach
Copy link
Contributor

ofZach commented Dec 22, 2015

thanks it's gonna take me a few days to get my gears cranking again. also it looks like this issue:

electron/packager#111

has been fixed upstream

max-mapper/extract-zip#6

so we might not even need shrinkwrap.... It was basically being used to point to a specific fix upstream. gonna check....

@mattfelsen
Copy link
Member Author

Gotcha. All good, I'm on a pg roll today 😎

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.

4 participants