Skip to content

Build blazor.*.js in CI; don't commit the artifacts #11592

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

Closed
SteveSandersonMS opened this issue Jun 26, 2019 · 10 comments
Closed

Build blazor.*.js in CI; don't commit the artifacts #11592

SteveSandersonMS opened this issue Jun 26, 2019 · 10 comments
Assignees
Labels
affected-few This issue impacts only small number of customers area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework severity-major This label is used by an internal tool task
Milestone

Comments

@SteveSandersonMS
Copy link
Member

At some point in the recent past, we started including blazor.*.js in source control. This was intended as a way of avoiding the Node dependency in some CI cases. However, it turns out that:

  • This is very damaging to productivity, as it forces us to shepherd our PRs through CI in series, not in parallel. Every commit that affects the blazor.*.js artifacts causes a merge conflict for every other such commit, even if they were only touching completely unrelated files. Some work items have been queued up for days due to this.
  • It's super annoying that sometimes you wait ages for a CI build result, and it fails claiming your output was out of date, even though in some cases that doesn't appear to be true. We haven't tracked down the exact conditions under which this occurs, though it might be something like when SignalR makes changes that overlap with the PR's lifetime.
  • It turns out to be completely unnecessary anyway. We've consulted with the build team (Doug/John, plus Chris regarding source build) and everyone agrees it's actually completely fine to use Node during CI builds, and it's already installed in all our environments except CentOS (where it can be installed via yum).

The build team has agreed to support us with figuring out how to make this change, though @dougbu is asking that someone from the Blazor crew actually do it.

@SteveSandersonMS SteveSandersonMS added task area-blazor Includes: Blazor, Razor Components labels Jun 26, 2019
@BrennanConroy
Copy link
Member

completely fine to use Node during CI builds

SignalR has been doing this since the beginning. If you need any help or guidance you can ask us or take a look at our npm builds. https://github.com/aspnet/AspNetCore/tree/master/src/SignalR/clients/ts

@JunTaoLuo
Copy link
Contributor

This has been completed in #12145

@JunTaoLuo JunTaoLuo removed the Done This issue has been fixed label Aug 5, 2019
@JunTaoLuo JunTaoLuo modified the milestones: 3.0.0-preview8, 3.1.0 Aug 5, 2019
@JunTaoLuo JunTaoLuo reopened this Aug 5, 2019
@JunTaoLuo
Copy link
Contributor

We can't actually do this given source build requirements. Though we have nodejs and yarn, we don't have the dependencies such as typescript available in source build and won't be able to compile our ts source into js artifacts. This is going to be tricky to resolve given our source build commitments.

@rynowak
Copy link
Member

rynowak commented Aug 5, 2019

We need to keep pushing on this somehow (post 3.0 OK). The current state us functional (we can make progress) but unacceptable for the team's productivity (JS changes can never be merged automatically and require manual intervention).

@mkArtakMSFT mkArtakMSFT modified the milestones: 3.1.0, 3.1.0-preview1 Aug 12, 2019
@mkArtakMSFT mkArtakMSFT added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 20, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: 3.1.0-preview2, 3.1.0 Oct 16, 2019
@mkArtakMSFT
Copy link
Contributor

Related #43715

@mkArtakMSFT
Copy link
Contributor

Blocked on #37398

@wtgodbe
Copy link
Member

wtgodbe commented Jan 9, 2024

Done by #53154

@wtgodbe wtgodbe closed this as completed Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework severity-major This label is used by an internal tool task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants