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

[kbn-pm] Include Kibana's transitive _projects_ in build #16813

Merged
merged 1 commit into from
Mar 12, 2018

Conversation

kimjoar
Copy link
Contributor

@kimjoar kimjoar commented Feb 19, 2018

I realized we already had the required info in the deps, so we don't need to rely on the kibana.build.skip field in the package.jsons. The includeTransitiveProjects helper could potentially enable some nice features as we move towards watching and stuff like that too.

Also, this makes it more obvious that only transitive projects of Kibana will are intended to work right now, as they are the only ones included when yarn is ran. Later on I think we might want to include other things in the build too, but then I think we should specifically include it through package.json instead, plus we need a solution for installing deps.

@kimjoar kimjoar added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 19, 2018
@kimjoar kimjoar changed the title Include Kibana's transitive _projects_ in build [kbn-pm] Include Kibana's transitive _projects_ in build Feb 19, 2018
@kimjoar kimjoar force-pushed the kbn-build/transitive-projects branch from 4ca4a6f to 539ea41 Compare February 20, 2018 12:38
@kimjoar kimjoar requested a review from spalger February 20, 2018 12:38
@kimjoar kimjoar force-pushed the kbn-build/transitive-projects branch 4 times, most recently from 529c5ab to bc7f375 Compare February 21, 2018 21:20
@kimjoar kimjoar removed the request for review from spalger February 21, 2018 22:03
@kimjoar kimjoar force-pushed the kbn-build/transitive-projects branch from bc7f375 to a8c1a14 Compare February 22, 2018 05:57
@elastic elastic deleted a comment from elasticmachine Feb 22, 2018
@elastic elastic deleted a comment from elasticmachine Feb 22, 2018
@elastic elastic deleted a comment from elasticmachine Feb 22, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kimjoar kimjoar force-pushed the kbn-build/transitive-projects branch 4 times, most recently from 374d49a to 7a7909a Compare February 22, 2018 07:23
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kimjoar kimjoar force-pushed the kbn-build/transitive-projects branch from 7a7909a to d6427b4 Compare March 9, 2018 12:19
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kimjoar
Copy link
Contributor Author

kimjoar commented Mar 9, 2018

@spalger Rebased and updated

@kimjoar kimjoar requested a review from spalger March 9, 2018 13:39
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

This works and skips the eslint stuff without needing config in each package, which is a good step, but could be improved in a future PR.

In the new build process we'll have multiple package.json files, one in each build output, and each will define slightly different projects necessary for the build. If buildProductionProjects() knew this it could read the package.json from the build output and only build the projects necessary for that build.

Even further, it could know that there are multiple build targets and only run each build step once and copy the output to each build target that needs it.

Alternatively, we can go back to the bulk of this being implemented in the build tasks and expose an API for the build tasks to get project info/graphs so it can do just the work it needs.

Like I said, we can do all this in subsequent prs if you like any of it.

@kimjoar
Copy link
Contributor Author

kimjoar commented Mar 12, 2018

Great input, @spalger. I like the idea of extracting build_production_projects.ts into the build tasks, and keeping @kbn/pm focused on the projects themselves, including exposing functions like includeTransitiveProjects.

@kimjoar kimjoar merged commit 48f8dc3 into elastic:master Mar 12, 2018
kimjoar added a commit to kimjoar/kibana that referenced this pull request Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.3.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants