Skip to content

Revert "Revert "Improve components infrastructure (#12145)" (#12679)" #12744

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

Merged
merged 7 commits into from
Aug 5, 2019

Conversation

JunTaoLuo
Copy link
Contributor

This reverts commit e2d57e2.

As per meeting discussion, this is the approach we would like to take for source build as well. However, there will likely be pieces of the infrastructure that needs to be modified (e.g. not redistributing yarn binaries) but we'll address those separately.

I'm adding the original changes to preview 9 for improving workflow.

Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Has anything changed in this since the original PR? If yes, where should we dive deeper?

@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jul 31, 2019
@JunTaoLuo JunTaoLuo added the tell-mode Indicates a PR which is being merged during tell-mode label Aug 3, 2019
@JunTaoLuo JunTaoLuo force-pushed the johluo/revert-revert branch from ea1bc88 to 4034ec2 Compare August 5, 2019 03:59
@rynowak
Copy link
Member

rynowak commented Aug 5, 2019

revert revert is the best kind of revert. Prove me wrong.

@JunTaoLuo
Copy link
Contributor Author

I'm going to update with a few more details on how I intend for this to work when all tests come back clean but I believe this is close to ready.

Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Still looks good.

@JunTaoLuo
Copy link
Contributor Author

Here's a few of the common scenarios and how it will work when you run build by default (i.e. without specifying BuildNodeJs or NoBuildNodeJS):

Scenario node installed no node
build.cmd in root builds dependent npm projects uses checked in assets if available, displays warning
build.cmd in src/Components builds dependent npm projects uses checked in assets if available, displays warning
dotnet build on Microsoft.AspNetCore.Components.Server builds dependent npm projects uses checked in assets if available, displays warning
yarn run build on Web.JS npmproj does not build dependent projects fails

With explicit specification of BuildNodeJs:
The dependent npm projects will be built if node is available on the path and the build will fail if it's not.

With explicit specification of NoBuildNodeJs:
The dependent npm projects will not be built and the JS artifacts from previous builds will be used. If the JS artifacts are not present (only the release versions are checked into source control) the build will fail

@halter73
Copy link
Member

halter73 commented Aug 5, 2019

@rynowak, @pranavkm calls your revert revert and raises to revert revert revert.

@rynowak
Copy link
Member

rynowak commented Aug 5, 2019

Dank.

@JunTaoLuo
Copy link
Contributor Author

Revertception
image

@JunTaoLuo JunTaoLuo merged commit 43350b5 into release/3.0 Aug 5, 2019
@ghost ghost deleted the johluo/revert-revert branch August 5, 2019 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants