Skip to content
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

build: rewrite static asset build #31912

Closed

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Mar 12, 2023

Description

Closes:

Merge considerations

This PR just adds the new build implementation. It does not remove the old implementation, and it does not update anything to point at the new implementation.

Next steps

Testing instructions

Preview without running

Just check this branch, and right on your local machine, run:

scripts/build-assets.sh --help

and

scripts/build-assets.sh --dry-run

Run the asset build and see the results in your browser

  • Install Tutor Nightly and set up a development environment.
  • Run the asset build with: tutor dev run -m <your/edx-platform/path> assets/build.sh
  • Run the platform with: tutor dev start -m <your/edx-platform/path>

Build Tutor images

(More detailed test instructions coming soon)

@kdmccormick kdmccormick force-pushed the kdmccormick/assets-build branch 2 times, most recently from 56323e0 to 700fc7b Compare March 15, 2023 20:05
@kdmccormick kdmccormick force-pushed the kdmccormick/assets-build branch 11 times, most recently from cdb55d4 to 76d2586 Compare April 20, 2023 22:08
@kdmccormick kdmccormick force-pushed the kdmccormick/assets-build branch 9 times, most recently from 7ee4080 to b74a541 Compare April 25, 2023 16:31
We want to compare the legacy and new outputs for SCSS compilation.
However, there is one silly difference between the two that we can
not reconcile:

* The legacy build (using Python `sass.compile`) adds source comments
  when in dev mode, but cannot output source maps.
* The new build (using CLI `sassc`) can only add source comments
  if it generates source maps.

So, for the purposes of making our comparison pass in CI, this commit:

* removes source_comments from legacy build, and
* removes --sourcemap from new build.

When we finally merge everything in, though, and no longer need this
comparison in CI, we will just enable source maps on the new build.
@kdmccormick
Copy link
Member Author

Closed in favor of #32823 and #32685

@kdmccormick kdmccormick deleted the kdmccormick/assets-build branch July 24, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant