Problem/Motivation

BABEL_ENV variable affects only Babel but CSS compiling and jQuery UI minifying scripts don’t use Babel in the conversion.

Proposed resolution

Removes BABEL_ENV from build:css, watch:css, and build:jqueryui.

Remaining tasks

Reviewing a patch.

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 babelafterpatch.png359.95 KBadmin@azhark.com
#4 babelbeforepatch.png338.29 KBadmin@azhark.com
#2 3212747-remove_unneeded_babel_env.patch1.2 KBtom konda

Comments

Tom Konda created an issue. See original summary.

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
StatusFileSize
new 3212747-remove_unneeded_babel_env.patch1.2 KB
tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Issue summary: View changes
Status: Active » Needs review
Issue tags: +frontend
admin@azhark.com’s picture

admin@azhark.com commented
Assigned: Unassigned » admin@azhark.com
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new babelbeforepatch.png338.29 KB
new babelafterpatch.png359.95 KB

After & Before applying patch.
It successfully removes BABEL_ENV from

  • build:css
  • watch:css
  • build:jqueryui

Before Path, package.json:
Before Patch

After Patch, package.json:
After Patch

admin@azhark.com’s picture

admin@azhark.com commented
Assigned: admin@azhark.com » Unassigned

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3212747-remove_unneeded_babel_env.patch, failed testing. View results

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community

Random test-failure #3214565: [random test failure] Random fail in BuildTestTest::testPortMany, back to RTBC per #4.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@adminazhar Thank you for looking into this issue.

Posting screenshots of your codebase or CLI does not advance the issue, since the automated testing infrastructure tells us whether the patch applies correctly.

So, I've removed the issue credit for that screenshot. In the future, you can get credit for issues by reading the issue to understand its purpose, and posting your review or testing of that purpose. Thank you!

See the issue credit guidelines for more information.

For example, with this issue the important that is to run the commands and show that the new commands result in no unexpected changes to css or jquery ui js.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

I've run the updated command locally and no changes are made. I agree that removing the unnecessary environment variable makes sense.

Committed cc5b85e and pushed to 9.3.x. Thanks!

  • alexpott committed cc5b85e on 9.3.x 
    Issue #3212747 by Tom Konda: Unneeded BABEL_ENV is configured on CSS...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.