mathiasvr / bluejay-configurator

Cross-platform application for Bluejay firmware flashing and configuration
https://github.com/mathiasvr/bluejay
GNU General Public License v3.0
38 stars 5 forks source link

gulp-appdmg should be optional #2

Closed stylesuxx closed 3 years ago

stylesuxx commented 3 years ago

This should be an optional dependency, otherwise deps can not be installed on linux for development. I have a modified package json and will prepare a PR.

mathiasvr commented 3 years ago

Right it used to be like that, but because they switched to yarn the old scripts didn't work on mac so I "fixed" it 😓

stylesuxx commented 3 years ago

Ah, OK - so we have a deadlock here? If we change it to optional, you can't build it, and if we leave it, I can't build it :-D For me it does not really matter, but in this situation I suggest simply adding a section to the readme explaining the situation for Linux.

mathiasvr commented 3 years ago

I think it's possible to make it optional, it's just the way it was done on blheli-configurator master did some npm install only on mac that didn't work with yarn. At least it would be better to remove it as a dependency and just install manually on mac.

mathiasvr commented 3 years ago

Actually, there is an open ~issue~ pr about gulp-appdmg not working on newer versions of mac so doesn't even make sense to try making it optional anyway. I'm fine with removing it and have mac users figure out how to build for themselves 😅

Btw the dmg thing is not needed run/debug the app.

Oh just realized it might not be possible to cross-compile to mac on other platforms? In that case it's a little more problematic, but I can still build the releases for now.

stylesuxx commented 3 years ago

I know it is not needed for debug - that's why I suggested to put it in the optionals - otherwise installation of the dependencies will fail on Linux.

And you are right, it is needed to build the bin for mac - and from what I understand it will only work on Mac anyway. So if you keep building the releases everything is fine and dandy - from what I understand you need to build on a Mac anyway to be able to build all bins...

So the conclusion is: We remove gulp-appdmg as a dependency and add a section to the Readme to install it if developing or building on mac.