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

Delete browserSandbox if false #75

Closed

Conversation

RobbieTheWagner
Copy link

There was no way to disable this if you were on Electron 5.x, this should allow users to set it to false to remove it.

@malept thoughts on something like this?

There was no way to disable this if you were on Electron 5.x, this should allow users to set it to `false` to remove it.
@welcome
Copy link

welcome bot commented May 14, 2020

Thanks for opening a pull request!

Here are some highlighted action items that will help get it across the finish line, from the
pull request guidelines:

  • Follow the JavaScript coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes in NEWS.md and other docs.
  • Include tests when adding/changing behavior.

Development and triage is community-driven, so please be patient and we will get back to you as soon as we can.

@RobbieTheWagner
Copy link
Author

@malept I have been told repeatedly by the Snapcraft guys now that sandbox is only allowed for vetted developers, which are only big publishers like Google Chrome, Brave, etc. It seems to be next to impossible to apply to be a vetted developer or be allowed to use sandbox. With this in mind, perhaps we should remove the option entirely?

@mahnunchik
Copy link

@malept It seems it is better to to use allow-sandbox: false according to the documentation: https://snapcraft.io/docs/browser-support-interface

@RobbieTheWagner
Copy link
Author

@mahnunchik this PR makes it configurable.

@codecov
Copy link

codecov bot commented Sep 3, 2020

Codecov Report

Merging #75 (d8ad2da) into master (7a4a4df) will decrease coverage by 0.39%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #75      +/-   ##
===========================================
- Coverage   100.00%   99.60%   -0.40%     
===========================================
  Files            8        8              
  Lines          253      255       +2     
===========================================
+ Hits           253      254       +1     
- Misses           0        1       +1     
Impacted Files Coverage Δ
src/yaml.js 99.12% <66.66%> (-0.88%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a4a4df...f1a7035. Read the comment docs.

@davidwinter
Copy link

Can this PR please be reviewed and merged if working? Currently snap builds are currently not possible with electron-forge, making it impossible to use as a tool.

@davidwinter
Copy link

@rwwagner90 any chance you could add tests to cover the change so that the codecov bot passes? Also, if you could merge in the latest changes, then perhaps we could get @malept to merge the PR in?

@RobbieTheWagner
Copy link
Author

I clicked the update button to get it up to date with master. I could try to add a test, but I am not 100% sure this is the right solution, it just illustrates what I think should be done. I would love input and direction from @malept here!

@RobbieTheWagner
Copy link
Author

@malept are we interested in making this repo work again or not? If not, I'll close the PR.

@RobbieTheWagner
Copy link
Author

It seems like there are no intentions of updating this package to actually work with snap, so if anyone else lands here looking for a fix for broken electron-forge setup, please use https://github.com/davidwinter/electron-forge-maker-snap

@Igloczek
Copy link

Igloczek commented May 7, 2024

Anyone know how to handle this topic now?

  • electron-forge-maker-snap is abandoned
  • @electron-forge/maker-snap do basically nothing, just wraps this package and pass arch, src and dest to it
  • this package is not handling the sandbox thingies properly on its own, since snap store automatically reject the builds
  • adding --no-sandbox makes no difference

are there any docs, guides or something else that describes how this stuff should be used?

from what i see, big bois like vs-code are not even trying to use any of these tools and just create a snapcraft.conf file on on their own and call snapcraft CLI directly

is that the only way, since everything else simply doesn't work or maybe it's working but only "if you know you know" mode?

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