electron-archive / grunt-electron-installer

Grunt plugin to build Windows installers for Electron apps
MIT License
398 stars 106 forks source link

Seperate from grunt #91

Closed lukeapage closed 8 years ago

lukeapage commented 8 years ago

it would be more useful to have this as 2 npm packages - a library that can create the electron installer and then packages for grunt and gulp that consume it.

Its already been forked into a gulp specific package here: https://github.com/Aluxian/electron-windows-installer But its diverged and I'd rather not fork again..

lukeapage commented 8 years ago

and another fork that has removed the grunt part: https://github.com/mongodb-js/electron-installer-squirrel-windows

havenchyk commented 8 years ago

Yep, I'm agree with you. BTW CoffeScript prevents other developers to contribute, but it's IMO.

lukeapage commented 8 years ago

Coffeescript is a separate discussion (but I do agree with you): https://discuss.atom.io/t/why-coffeescript/131 Its in the github style guide, so I don't see that changing any time soon.

havenchyk commented 8 years ago

It's not possible to provide PR without the decision from core team, so guys @lukeapage @kevinsawicki, what do you think?

lukeapage commented 8 years ago

yeah I'm happy to do the work.. but we need help from the core team @kevinsawicki @zcbenz @paulcbetts ? first we need a new repo atom/electron-installer - If you create it empty I don't mind making a PR to it. After that is merged and npm published a new PR can make this repo use that package.

Would of course be good to have any PR's merged before its split.

anaisbetts commented 8 years ago

This is on my list, to write a library for this that isn't tied to grunt and does all of the things associated with packaging. My list is long though :-/

kevinsawicki commented 8 years ago

it would be more useful to have this as 2 npm packages - a library that can create the electron installer and then packages for grunt and gulp that consume it.

:+1: Sounds good to me, thanks for bringing this up.

first we need a new repo atom/electron-installer - If you create it empty I don't mind making a PR to it. After that is merged and npm published a new PR can make this repo use that package.

Awesome, thanks for helping out with this, I'll create the repo today.

lukeapage commented 8 years ago

Thanks.

lukeapage commented 8 years ago

@kevinsawicki if you push the current repo to electron-installer as a starting point we wont lose history. Btw i wont remove coffee, someone else can do that in a seperate pr

kevinsawicki commented 8 years ago

@lukeapage New repository created from this one over in https://github.com/electronjs/windows-installer

lukeapage commented 8 years ago

Thanks, will get a pr done in the next week.

havenchyk commented 8 years ago

@lukeapage I didn't get it. Would you like to remove coffee?

lukeapage commented 8 years ago

Yes but thats a seperate concern..

havenchyk commented 8 years ago

AFAIK, https://github.com/mongodb-js/electron-installer-squirrel-windows contains same code, but without the coffee and grunt. Maybe we can involve @imlucas in it? It's not reasonable to do work twice.

lukeapage commented 8 years ago

Feel free to cherry pick the commits and make a pr yourself (am in an airport so it will be a while before i check - but i would look too), but im sure the maintainers would appreciate it done as 2 prs. This will stop any work being lost since it was forked.

imlucas commented 8 years ago

:wave: this is rad. for compass (the team at MongoDB i work on), we've been porting as many of atom's grunt tasks into small modules as possible. there are quite a few of these already with several others waiting to be cleaned up and published. quite fortunately, i'm working on autoupdates for compass over the new couple of weeks. doesn't seem the timing could be any better :D

@paulcbetts: i believe we've been thinking and doing very similar things... quite curious for your thoughts!

@havenchyk: how can I help? something in the very short-term you need specifically?

lukeapage commented 8 years ago

I started this here: https://github.com/electronjs/windows-installer/pull/1 I took a look at electron-installer-squirrel-windows but 1) its alot of commits behind 2) the initial commits are massive so I can't cherry pick them

I think it makes sense to do this in stages, so after removing grunt, then move any PR's outstanding from here to there and make a PR to make this use the new module.

removing coffee is a seperate concern and can be done any time after successfully transitioning to a grunt-less version.

@imlucas possibly after this is done (so long term future) you'd consider deprecating your module and making PR's for anything additional you added that isn't in this repo.

havenchyk commented 8 years ago

@lukeapage well done!

imlucas commented 8 years ago

@lukeapage sounds good to me!

as you can see from trying to cherrypick from mongodb-js/electron-installer-squirrel-windows, I would absolutely love to deprecate it as its been a real challenge to keep in sync w/ the upstream :/ you also may have noticed looking through that code all of the TODO's so not adverse at all to actually rewriting all of this.

re: removing coffee specifically: quite curious what you have in mind for this as we've learned quite a bit in getting all of these mongodb-js/electron-installer* modules into production, but perhaps we should just discuss when we get there?

lukeapage commented 8 years ago

PR to make a new seperate module: https://github.com/electronjs/windows-installer/pull/1 PR to move this to use that: https://github.com/atom/grunt-electron-installer/pull/99

develar commented 8 years ago

I use windows-installer in electron-complete-builder and really want to help. Because I want to get rid of all *sync methods. So — do you have plans to use ES6 (async/await)? If not, will such change be accepted? Or, maybe even in TS (sample async/await code in TS — https://github.com/develar/electron-complete-builder/blob/master/src/codeSign.ts)

I am ready to prepare pull request (once https://github.com/electronjs/windows-installer/pull/1 will be accepted to avoid merge conflicts).

anaisbetts commented 8 years ago

@develar I already did it, see electronjs/windows-installer#2)

havenchyk commented 8 years ago

The work in progress, I'm waiting for #104 and #107 to be merged. Is there any blockers? /cc @kevinsawicki @paulcbetts

havenchyk commented 8 years ago

Done, can be closed.