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

[B5] Update core report modules to ESM format and fix datatables imports for bootstrap 5 #35656

Merged
merged 5 commits into from
Jan 23, 2025

Conversation

biyeun
Copy link
Member

@biyeun biyeun commented Jan 22, 2025

Technical Summary

Updates base.js, tabular.js and datatables_config.js to ESM format

Safety Assurance

Safety story

Safe change. Not used by any production pages (no bootstrap 5 reports yet)

Automated test coverage

N/A

QA Plan

Not needed

Rollback instructions

unchecking due to bootstrap 5 diffs. late reverts can cause issues.

  • 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

@biyeun biyeun added the product/invisible Change has no end-user visible impact label Jan 22, 2025
@biyeun biyeun requested review from orangejenny and a team January 22, 2025 17:18
@biyeun
Copy link
Member Author

biyeun commented Jan 22, 2025

@orangejenny interesting...tests are showing webpack build errors for bootstrap 3 files...

@biyeun
Copy link
Member Author

biyeun commented Jan 22, 2025

Also, diffs are a reason why i didn't separate these out in the first place

@biyeun biyeun force-pushed the bmb/b5-reports-esm branch from 92e5677 to 426a708 Compare January 22, 2025 17:34
@biyeun biyeun merged commit d1c87b7 into master Jan 23, 2025
12 of 13 checks passed
@biyeun biyeun deleted the bmb/b5-reports-esm branch January 23, 2025 14:52
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