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.

Sign up for GitHub

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

Jump to bottom

Build and Test both the Dev and Production Webpack Asset Process #34834

Closed
feanil opened this issue May 21, 2024 · 1 comment · Fixed by #34836
Closed

Build and Test both the Dev and Production Webpack Asset Process #34834

feanil opened this issue May 21, 2024 · 1 comment · Fixed by #34836
Assignees
@kdmccormick
Labels
maintenance Routine upkeep necessary for the health of the platform

Comments

@feanil
Copy link
Contributor

feanil commented May 21, 2024 •
edited
Loading

Context

The static assets check workflow only builds the dev version of the webpack assets and as such does not test that the production webpack config will build properly. This is partly due to the fact that the assets paver commands make it difficult to test the production webpack build without configuring a more production like edx-platform python settings.

Furthermore, the paver commands are deprecated and alternatives already exist which make it easier to build either the dev or production webpack assets.

Acceptance Criteria

  • Update the statics assets check workflow to use the new non paver commands to compile assets.
  • Update the workflow to build both the dev and production assets.
@feanil feanil added the maintenance Routine upkeep necessary for the health of the platform label May 21, 2024
@feanil feanil mentioned this issue May 21, 2024
Open
@kdmccormick kdmccormick self-assigned this May 21, 2024
@kdmccormick kdmccormick mentioned this issue May 21, 2024
Merged
@kdmccormick
Copy link
Member

Thanks for the reminder. Now that the DEPR is accepted, it's good time to switch the CI check from Paver to the new thing.

Ready for review, assuming tests pass: #34836

kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue May 21, 2024
@kdmccormick
test: check both dev and prod asset builds (with npm run)
aaba274 
This switches the static asset check over from the deprecated
paver commands to the new 'npm run build' command. Doing so allows us
to check both the prod AND dev build, whereas before we were only
checking the prod build.

Please note that, as before, the "check" is only ensuring that the build
returns 0. It is not checking the contents of the build output.

Closes: openedx#34834
kdmccormick added a commit that referenced this issue May 22, 2024
@kdmccormick
test: check both dev and prod asset builds using npm run (#34836)
69e5fa4 
This switches the static asset check over from the deprecated
paver commands to the new 'npm run build' command. Doing so allows us
to check both the prod AND dev build, whereas before we were only
checking the prod build.

Please note that, as before, the "check" is only ensuring that the build
returns 0. It is not checking the contents of the build output.

Closes: #34834
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kdmccormick kdmccormick

Labels
maintenance Routine upkeep necessary for the health of the platform
Projects
Aximprovements Team
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: check both dev and prod asset builds (with npm run) kdmccormick/edx-platform
2 participants
@feanil @kdmccormick