Skip to content

feat(@angular-devkit/build-angular): default to NodeJS value for pres…#16648

Merged
dgp1130 merged 1 commit into
angular:masterfrom
Toxicable:preserve-symlink-nodejs
Mar 23, 2020
Merged

feat(@angular-devkit/build-angular): default to NodeJS value for pres…#16648
dgp1130 merged 1 commit into
angular:masterfrom
Toxicable:preserve-symlink-nodejs

Conversation

@Toxicable
Copy link
Copy Markdown

…erveSymlinks

Under bazel preserveSymlinks would have to be set in two different places, this makes it so it only has to be set once by using the value from NodeJS if it's set.

@Toxicable
Copy link
Copy Markdown
Author

Not sure if this is the right place to put this config, let me know if there's a better place to set it.

Comment thread packages/angular_devkit/build_angular/src/browser/index.ts Outdated
@clydin
Copy link
Copy Markdown
Member

clydin commented Jan 17, 2020

I’m ok with using it as a default. However, it would be a breaking change since the default would be different for some people. This could manifest in either broken builds or builds containing different dependencies than anticipated or expected. As a feature, this would probably fall into the v10 timeframe.

@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch 2 times, most recently from e3699cb to f5d55dd Compare January 17, 2020 05:52
@Toxicable
Copy link
Copy Markdown
Author

@clydin Sounds good to me, how would we make sure to come back and check this when there's a v10 branch?

Comment thread docs/documentation/1-x/angular-cli.md Outdated
Comment thread packages/angular_devkit/core/node/resolve.ts
@alan-agius4
Copy link
Copy Markdown
Collaborator

@Toxicable, I can create a v10 milestone and assign this Pr to it.

@alan-agius4 alan-agius4 added this to the V10-candidates milestone Jan 17, 2020
@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch 2 times, most recently from 826bdc6 to 6105bf4 Compare January 17, 2020 07:16
@alan-agius4 alan-agius4 changed the title feat(@angualr-devkit/build-angular): default to NodeJS value for pres… feat(@angular-devkit/build-angular): default to NodeJS value for pres… Jan 17, 2020
@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch from 6105bf4 to 3282882 Compare January 17, 2020 19:27
Comment thread packages/schematics/angular/utility/config.ts Outdated
@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch from 3282882 to 611a57c Compare January 17, 2020 19:29
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

Thanks Fabian! LGTM.

@alan-agius4 alan-agius4 self-assigned this Jan 17, 2020
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release and removed PR state: blocked labels Mar 23, 2020
@dgp1130 dgp1130 merged commit bc5ce39 into angular:master Mar 23, 2020
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Apr 23, 2020
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 target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants