Skip to content

Conversation

@devversion
Copy link
Member

@devversion devversion commented Nov 16, 2018

  • Since bazel-contrib/rules_nodejs@1f97eb6 landed, we can safely remove the workaround by updating the NodeJS and TypeScript Bazel rules.
  • Also we are creating our own getFileContent testing method because it's not guaranteed that the internal testing utilities from @schematics/angular always stay the same. Additionally this works around an issue where test folders are excluded in the Bazel managed deps by default.

Blocked on angular/angular#27138 being merged.

@devversion devversion added pr: merge safe target: patch This PR is targeted for the next patch release labels Nov 16, 2018
@devversion devversion requested a review from jelbourn as a code owner November 16, 2018 16:51
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 16, 2018
@devversion devversion added the blocked This issue is blocked by some external factor, such as a prerequisite PR label Nov 16, 2018
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

@gregmagolan
Copy link
Contributor

gregmagolan commented Nov 17, 2018

Just a note that rules_nodejs 0.16.1 won't work with material but the latest commit will (symlink issue fixed by bazel-contrib/rules_nodejs#422). This was due to the test folder being filtered out that was needed in the @schematics/angular package.

You can either update this PR to that commit or Alex should tag 0.16.2 shortly.

With the rules_nodejs update, the update to yarn 1.10+ is possible as well which supports the integrity field:

node_repositories(
  # For deterministic builds, specify explicit NodeJS and Yarn versions.
  node_version = "10.10.0",
  # Use latest yarn version to support integrity field (added in yarn 1.10)
  yarn_version = "1.12.1",
)

@devversion devversion force-pushed the build/remove-windows-runfile-workaround branch from 1ba79e3 to f85f68c Compare November 19, 2018 16:51
* Since bazel-contrib/rules_nodejs@1f97eb6 landed, we can safely remove the workaround by updating the NodeJS and TypeScript Bazel rules.
* Also we are creating our own `getFileContent` testing method because it's not guaranteed that the internal testing utilities from `@schematics/angular` always stay the same. Additionally this works around an issue where `test` folders are excluded in the Bazel managed deps by default.
@devversion devversion force-pushed the build/remove-windows-runfile-workaround branch from f85f68c to 9c4056b Compare November 23, 2018 19:24
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed blocked This issue is blocked by some external factor, such as a prerequisite PR labels Nov 23, 2018
@devversion
Copy link
Member Author

devversion commented Nov 23, 2018

Rebased. This is ready now.

@jelbourn jelbourn merged commit fccbfb5 into angular:master Nov 27, 2018
jelbourn pushed a commit that referenced this pull request Dec 3, 2018
* Since bazel-contrib/rules_nodejs@1f97eb6 landed, we can safely remove the workaround by updating the NodeJS and TypeScript Bazel rules.
* Also we are creating our own `getFileContent` testing method because it's not guaranteed that the internal testing utilities from `@schematics/angular` always stay the same. Additionally this works around an issue where `test` folders are excluded in the Bazel managed deps by default.
josephperrott pushed a commit to josephperrott/components that referenced this pull request Jan 14, 2019
* Since bazel-contrib/rules_nodejs@1f97eb6 landed, we can safely remove the workaround by updating the NodeJS and TypeScript Bazel rules.
* Also we are creating our own `getFileContent` testing method because it's not guaranteed that the internal testing utilities from `@schematics/angular` always stay the same. Additionally this works around an issue where `test` folders are excluded in the Bazel managed deps by default.
@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