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

Cleaned up mocha/base.html #36081

Merged
merged 1 commit into from
Mar 26, 2025
Merged

Cleaned up mocha/base.html #36081

merged 1 commit into from
Mar 26, 2025

Conversation

orangejenny
Copy link
Contributor

Technical Summary

All js tests now use webpack. This PR removes the last bits of code used to set up requirejs tests or tests not using a bundler.

Safety Assurance

Safety story

It's all tests.

Automated test coverage

It's all tests.

QA Plan

no

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@orangejenny orangejenny added the product/invisible Change has no end-user visible impact label Mar 26, 2025
Copy link
Contributor

@jingcheng16 jingcheng16 left a comment

Choose a reason for hiding this comment

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

I believe you know what you're removing. 🥹 As someone doesn't have any context on mocha and never wrote any js tests, only have two questions, mainly to make sure this two lines are not removed accidentally. I'm happy to give an approval.

<link href="{% static 'mocha/mocha.css' %}" type="text/css" rel="stylesheet"/>

<script>
window.USE_BOOTSTRAP5 = {{ use_bootstrap5|BOOL }};
</script>

{% block stylesheets %}{% endblock %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why stylesheets is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There just aren't any tests using it. This isn't directly related to requirejs/webpack changes.

@orangejenny
Copy link
Contributor Author

@jingcheng16 Good questions, responded. This is a safe PR so I'm going to merge, but let me know if you have additional questions or comments.

@orangejenny orangejenny merged commit f3efb32 into master Mar 26, 2025
13 checks passed
@orangejenny orangejenny deleted the jls/mocha-cleanup branch March 26, 2025 20:37
@jingcheng16
Copy link
Contributor

@orangejenny Thank you for always being willing to provide detailed explanation with code examples!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/invisible Change has no end-user visible impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants