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

Add platforms options to meteor build: #11437

Conversation

matheusccastroo
Copy link
Contributor

This commit adds the "--platforms" flag to meteor build command. This allows the user to specify which platforms he want to build. For example, if we pass only android to it, we will build only android and web.cordova.

I think this is pretty useful when we want to build a specific mobile app. For example, I want to build only for android and not for iOS (but I may still build for iOS on another time, so removing and adding the platform is more work to do).

Usage example: meteor build . --platforms=android.

Note: if no platforms flag is used, it will do the build like it always did.

Related to issue #63 (meteor/meteor-feature-requests#63)

This commit adds the "--platforms" flag to meteor build command. This allows the user to specify which platforms he want to build and build it only. For example, if we pass only android to it, we will build only android and web.cordova.

Related to issue meteor#63 (meteor/meteor-feature-requests#63)
@CLAassistant
Copy link

CLAassistant commented May 17, 2021

CLA assistant check
All committers have signed the CLA.

@matheusccastroo matheusccastroo marked this pull request as ready for review May 17, 2021 18:58
Copy link
Contributor

@renanccastro renanccastro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to also have documentation in our meteor/docs repo about the new flag

Copy link
Collaborator

@StorytellerCZ StorytellerCZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a corresponding documentation. We'll get this into release 2.3 and release a beta in a day or two.

@matheusccastroo
Copy link
Contributor Author

@renanccastro @StorytellerCZ PR for documentation created: meteor/docs#720

@StorytellerCZ StorytellerCZ changed the base branch from devel to release-2.3 May 17, 2021 19:33
@StorytellerCZ StorytellerCZ merged commit 319a6c9 into meteor:release-2.3 May 17, 2021
Copy link
Collaborator

@filipenevola filipenevola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it would be better to have --exclude-archs support also in the build command, instead of a new flag platforms.

Any special reason to have a new flag here?

@@ -1014,6 +1015,19 @@ var buildCommand = function (options) {
// _bundleOnly implies serverOnly
const serverOnly = options._bundleOnly || !!options['server-only'];

let selectedPlatforms;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to initialize with null

const filteredArchs = projectContext.platformList.getWebArchs().filter((arch) => selectedPlatforms.includes(arch));

if (! _.isEmpty(cordovaPlatforms)) {
if (filteredArchs.indexOf("web.cordova") < 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be includes instead of indexOf

@StorytellerCZ StorytellerCZ added this to the Release 2.3 milestone May 17, 2021
@matheusccastroo
Copy link
Contributor Author

I believe it would be better to have --exclude-archs support also in the build command, instead of a new flag platforms.

Any special reason to have a new flag here?

@filipenevola Not really, I was just following what was here #7144.

However, thinking again, I think --platforms is more intuitive to use, as you only need to specify what you want to have, and not what you want to exclude. In the end, I think it's just a matter of preference.

@filipenevola
Copy link
Collaborator

I did small changes here 839a69d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants