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.

Sign up for GitHub

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

Jump to bottom

v5 Electron Support (5-8) #2872

Open
wants to merge 81 commits into
base: v5
Choose a base branch
from
Open

v5 Electron Support (5-8) #2872

wants to merge 81 commits into from

Conversation

glenn2223
Copy link

Adding support for electron v5-8. PR'ing to v5 due to node-gyp bump

@glenn2223
Support upto Electron 8 (Windows ONLY)
f57f4b4 
Bringing support for Electron 5, 6, 7 & 8 (windows x32 & x64 only)

Looking to expand to all other platforms but can't myself. See https://github.com/sass/node-sass/tree/master/.github/Electron-support.md
@glenn2223
Docs to expand on Electron Support
9c5aeb6 
Added doc to explain steps required for electron support
@glenn2223
Update Electron-Support.md
7f5b6bf
@glenn2223
Update Electron-Support.md
90e697d
@glenn2223
Reverted isSupportedEnvironment
e6b1a43 
Reverted isSupportedEnvironment back to it's original state and removed the function for detecting current electron support
@glenn2223
Create electron-win.yml
0661d3b
@glenn2223
Delete Electron-Support.md
92b7ef0 
But mor ereading and we can do it without electron-rebuild
@glenn2223
Removed bindings
1760a4c
@glenn2223
Merge branch 'master' of https://github.com/glenn2223/node-sass
ed1c343
@glenn2223
Update electron-win.yml
97a903b 
Targeted electron versions to stop fails
Added artifacts for testing
@glenn2223
Updated node-gyp to 6.1.0
2bc2261 
For electron building
@glenn2223
Renamed artifacts to almost match
5fe05fa 
Need to replace electron version with underlying node shown in a comment after each version number
@glenn2223
Workflow changes
80a5070 
Added new workflows and updated windows version
@glenn2223
Update electron-linux.yml
7368214
@glenn2223
Workflow updates
f3aaea6 
Oops
@glenn2223
Workflow updates
18687aa 
Windows ignorance and plain idiocy
@glenn2223
Workflow updates
499e066
@glenn2223
Fixes
9642e7b 
Skipping npm test as they always fail
Removed artifacts for .pdb on linus and osx
Added extra step for ia32 linux
@glenn2223
Update electron-linux.yml
d95fc8a
@glenn2223
Update electron-linux.yml
9d01660
@glenn2223
Update electron-linux.yml
69ddf4c
@glenn2223
Update electron-linux.yml
2cc27f0
@glenn2223
Update electron-linux.yml
480890d
@glenn2223
Update electron-linux.yml
72ed801
@glenn2223
Dropped x32 from linux workflow
138d277
@glenn2223
Reverted to electron rebuild
165f68a
@glenn2223
Merge branch 'master' of https://github.com/glenn2223/node-sass
2eb370e
@glenn2223
Update electron-win.yml
ed333ad
@glenn2223
Update workflows
4da3d46
@glenn2223
Workflow updates
17cfbc0
@glenn2223 glenn2223 mentioned this pull request Mar 23, 2020
Closed
@saper
Copy link
Member

saper commented Mar 23, 2020

Thank you @glenn2223 for your patience! I'll check if my build machinery can build node-sass for electron as well.

@glenn2223
Copy link
Author

glenn2223 commented Mar 23, 2020 •
edited

Ref: Commit Fix: Extensions don't honour argument arch

Test 1: Step 5: Line 4 setting arch as ia32, Step 5: Line 447 save path uses x64
Test 2: Success

Thanks @saper

nschonni
nschonni reviewed Apr 10, 2020
View reviewed changes
Copy link
Contributor

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

This seems to have a lot more random formatting churn and changes to the overall build setup than the last PR. What has changed that required this?

.github/workflows/electron-linux.yml Outdated Show resolved Hide resolved
@glenn2223
Copy link
Author

This seems to have a lot more random formatting churn and changes to the overall build setup than the last PR. What has changed that required this?

  • Formatting:
    Sorry I was working on another project at the same time so I just automatically hit my format shortcut. I can revert this if you wish?
  • Fix: (Fix: Extensions don't honour argument target-arch)
    Did you want this on another PR for this?
  • Electron targeting:
    I wasn't able to find a way to run electron commands (for each version) from the CI so I added a --electron-version argument. This added a function and expanded a couple of lines that I've added. This also has the added benefit of targeting an electron build without electron

@glenn2223
Small revision
3216467 
Spelling correction
Workflow update
Reverted stdio (sorry, was trying to see if I could get a clearer error for an issue I was facing)
@glenn2223
Update electron-linux.yml
b616802
@glenn2223
Update electron-osx.yml
7abdec3
@glenn2223
Update electron-linux.yml
d9e3870
@glenn2223
Update electron-win.yml
a34d22b
@glenn2223
Update yml
0a905c5
@glenn2223
Update extensions.js
7fa8d75
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nschonni nschonni nschonni left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@glenn2223 @saper @nschonni