Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

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

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
Added doc to explain steps required for electron support
Reverted isSupportedEnvironment back to it's original state and removed the function for detecting current electron support
But mor ereading and we can do it without electron-rebuild
Targeted electron versions to stop fails
Added artifacts for testing
For electron building
Need to replace electron version with underlying node shown in a comment after each version number
Added new workflows and updated windows version
Windows ignorance and plain idiocy
Skipping npm test as they always fail
Removed artifacts for .pdb on linus and osx
Added extra step for ia32 linux
@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

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

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

Spelling correction
Workflow update
Reverted stdio (sorry, was trying to see if I could get a clearer error for an issue I was facing)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants