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

Refactoring of Build Process #2120

Merged
merged 83 commits into from
Jul 13, 2017
Merged

Refactoring of Build Process #2120

merged 83 commits into from
Jul 13, 2017

Conversation

klappy
Copy link
Contributor

@klappy klappy commented Jul 9, 2017

This pull request addresses:

This PR addresses issues with the Build Process.
The build process is now lighter, less code, mostly a few config files.

  • Mac x64 builds (dmg)
  • Windows x86 builds (exe)
  • Testing on Linux x64 but no builds
  • Leveraging Electron-Forge.
  • Moved some essential files to be compatible and simplify the build process.
  • Updated references to essential files that were moved.
  • Non-destructive builds.

How to test this pull request:

  • Delete node_modules folder from project.
  • Run npm install
  • Run npm test to make sure all tests still pass.
  • Run npm start to make sure app still starts and runs.
  • Load projects and tools to ensure all looks well.
  • Run npm make on Mac to build the app.
  • Run the generated app in the out/make to ensure local build works.
  • Run git status to ensure the build was not destructive.

If Questions aren't answered in documentation let me know so it can be updated: https://github.com/unfoldingWord-dev/translationCore/wiki/Distributing-tC:-Package-Build-Publish


This change is Reviewable

klappy added 29 commits July 7, 2017 15:36
@klappy klappy added this to the tC Sprint #35 milestone Jul 9, 2017
@RoyalSix
Copy link

after running npm run make I got this error
`Jays-MBP:translationCore jay$ npm run make

translationCore@0.7.1-beta.55 make /Users/jay/Desktop/translationCore
electron-forge make

✔ Checking your system
✔ Resolving Forge Config
We need to package your application before we can make it
✔ Preparing to Package Application for arch: x64
✔ Compiling Application
✔ Preparing native dependencies: 3 / 3
✔ Packaging Application
Making for the following targets:
⠋ Making for target: dmg - On platform: darwin - For arch: x64
An unhandled exception has occurred inside Forge:
Module version mismatch. Expected 48, got 53.
Error: Module version mismatch. Expected 48, got 53.
at Error (native)
at Object.Module._extensions..node (module.js:597:18)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)
at Object. (/Users/jay/Desktop/translationCore/node_modules/macos-alias/lib/create.js:7:13)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)
at Object. (/Users/jay/Desktop/translationCore/node_modules/macos-alias/index.js:1:80)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! translationCore@0.7.1-beta.55 make: electron-forge make
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the translationCore@0.7.1-beta.55 make script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR! /Users/jay/.npm/_logs/2017-07-10T18_46_26_952Z-debug.log`

@mannycolon
Copy link
Contributor

Reviewed 4 of 38 files at r1, 4 of 4 files at r3.
Review status: 8 of 33 files reviewed at latest revision, 1 unresolved discussion.


package.json, line 24 at r3 (raw file):

  "repository": {
    "type": "git",
    "url": "git+https://github.com/klappy/translationCore.git"

do we want the code to point to your repo


Comments from Reviewable

@mannycolon
Copy link
Contributor

:lgtm:


Reviewed 29 of 38 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@klappy
Copy link
Contributor Author

klappy commented Jul 11, 2017

@RoyalSix: I added a section in the documentation to follow for Electron-Forge installation. It covers deleting node_modules folder, installing e-f, npm install and you should be good.


Comments from Reviewable

@mannycolon
Copy link
Contributor

@klappy I was thinking that maybe we need to test this branch in a Windows environment

@klappy
Copy link
Contributor Author

klappy commented Jul 11, 2017

@mannycolon I have been testing on Windows concurrently. The build process isn't ready for Windows yet though. The PR is focused on Mac builds currently as well as my progress for Windows.

Once we get this merged we can have builds for QA on Mac.

@RoyalSix
Copy link

RoyalSix commented Jul 12, 2017

Looks like it had some trouble getting the loader image for tC when I did npm start but the build finds it just fine

screen shot 2017-07-11 at 6 04 32 pm

@RoyalSix
Copy link

updated comment^^

@klappy
Copy link
Contributor Author

klappy commented Jul 12, 2017

@RoyalSix: Nice catch. I'm fixing that reference and any others I can find.

@klappy
Copy link
Contributor Author

klappy commented Jul 12, 2017

@RoyalSix: Let me know if you find anything else.

@klappy
Copy link
Contributor Author

klappy commented Jul 13, 2017

@RoyalSix @mannycolon: All issues are resolved and updated with latest develop branch. Is there anything else you need to merge this? Each time we merge into develop branch this process gets harder the longer we wait.

@mannycolon mannycolon merged commit cc48c60 into unfoldingWord:develop Jul 13, 2017
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