Skip to content

build: no longer require updating multiple package.json files for bazel #14391

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

Conversation

devversion
Copy link
Member

  • Replaces second "package.json" file with a small workspace that just aliases to the actual dependencies provided in the "@matdeps" workspace. This speeds up the analysis phase and also makes debugging easier.
  • Also proxies the "ng_package" so that it always includes the proper README.md file. Also this means that we can always replace the version placeholders. Similar to angular/angular.

Similar to angular/angular@68074df. Credits to @gregmagolan for this.

We just need a bit more work because we don't overwrite the default compiler, xi18n and packager targets to the actual source files (this is special to Angular because those contain the sources)

@devversion devversion added pr: merge safe target: patch This PR is targeted for the next patch release labels Dec 5, 2018
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 5, 2018
@devversion devversion force-pushed the build/no-longer-require-updating-multiple-package-json-bazel branch from bf0514e to b056693 Compare December 5, 2018 20:20
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -5,6 +5,7 @@ load("//tools:defaults.bzl", "ts_library")
exports_files([
"bazel-tsconfig-build.json",
"bazel-tsconfig-test.json",
"README.md",
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but we should create separate npm README files for each of our packages

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Should these be pretty much similar to the current one but just have the "correct" package name title and refer to the proper URL for the sources?

Copy link
Member

Choose a reason for hiding this comment

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

I created an issue and assigned it to myself for this.

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Dec 6, 2018
* Replaces second "package.json" file with a small workspace that just aliases to the actual dependencies provided in the "@matdeps" workspace. This speeds up the analysis phase and also makes debugging easier.
@devversion devversion force-pushed the build/no-longer-require-updating-multiple-package-json-bazel branch from b056693 to 3d8e7d5 Compare December 6, 2018 11:42
@mmalerba mmalerba merged commit 9067fd3 into angular:master Dec 6, 2018
josephperrott pushed a commit to josephperrott/components that referenced this pull request Jan 14, 2019
…el (angular#14391)

* Replaces second "package.json" file with a small workspace that just aliases to the actual dependencies provided in the "@matdeps" workspace. This speeds up the analysis phase and also makes debugging easier.
* Also proxies the "ng_package" so that it always includes the proper `README.md` file. Also this means that we can always replace the version placeholders. Similar to `angular/angular`.

Similar to angular/angular@68074df. Credits to @gregmagolan for this.

We just need a bit more work because we don't overwrite the default `compiler`, `xi18n` and `packager` targets to the actual source files (this is special to Angular because those contain the sources)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants