remove osx32 from the default list of platforms #439
Conversation
|
@preaction thanks for this. The default is mentioned in the README too. Could you update that as well please? |
|
Done |
lib/index.js
Outdated
| if(typeof options.platforms != 'undefined'){ | ||
| if(options.platforms.indexOf('osx') >= 0){ | ||
| options.platforms.splice(options.platforms.indexOf('osx'), 1, 'osx32', 'osx64'); | ||
| options.platforms.splice(options.platforms.indexOf('osx'), 1, 'osx64'); |
There was a problem hiding this comment.
Sorry now, I'm just looking at this again and thinking this is actually a breaking change. I.e. it would break people's builds relying on it. So if it was merged, it would require a major version bump (to 4.0.0), which would then require people to update their package.json / reinstall. I'm not sure this is worth it. Maybe we could not change this part (osx -> osx32, osx64), merge the rest, and wait until a 4.0.0 to include this specific change. What do you think?
There was a problem hiding this comment.
That's wholly up to you. I would agree that it's a breaking change, but in that same respect all of it is: osx32 used to be built by default, and now it won't be. I'd think that the opt-in to all OSX platforms with --platforms osx is more explicit than using the default, and more likely to cause problems with being changed (which I imagine is what you're thinking).
So, I'll remove this bit presently.
There was a problem hiding this comment.
Ugh, you're right about it all being a breaking change :/
I love merging stuff from new contributors but maybe we should wait. We can both keep an eye out for more breaking changes so we can merge this ASAP 😛
There was a problem hiding this comment.
I mean, the reason I would feel confident about making the changes that are left is because NW.js itself has dropped support for the platform: osx32 cannot work if you upgrade NW.js. If they want nw-builder to work with older versions of NW.js, they can update their build scripts to pass in the right --platform flag. But still totally up to you.
There was a problem hiding this comment.
OK. I'm so easily convinced 😁
But still totally up to you.
I'll direct any hate your way 😛.
Since 32-bit OSX isn't supported by NW.js anymore (because OSX hasn't been released for 32-bit platforms since 2011), it's more friendly to new developers to not give them an error using the default `nwbuild` configuration.
Since 32-bit OSX isn't supported by NW.js anymore (because OSX hasn't been released for 32-bit platforms since 2011), it's more friendly to new developers to not give them an error using the default
nwbuildconfiguration.This appears that it will conflict with #403, so if that gets merged I can rebase this.