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: use shared browserslist config #495

Closed

Conversation

adamstankiewicz
Copy link
Member

  • Removes custom browserslist configuration in favor of using a shared configuration.
  • Removes is-es5 check in CI since ES5 was only needed for IE 11 support which is dropped. The supported browsers defined by the shared configuration all natively support ES6.

This change reduces the resultant asset bundle size.

@adamstankiewicz adamstankiewicz requested a review from a team November 3, 2021 12:03
@codecov-commenter
Copy link

Codecov Report

Merging #495 (a6b81cd) into master (3da88dd) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #495   +/-   ##
=======================================
  Coverage   60.37%   60.37%           
=======================================
  Files          45       45           
  Lines         795      795           
  Branches      146      146           
=======================================
  Hits          480      480           
  Misses        305      305           
  Partials       10       10           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7948812...a6b81cd. Read the comment docs.

@Jawayria Jawayria force-pushed the astankiewicz/shared-browserslist-config branch from a6b81cd to dc7f492 Compare June 2, 2022 16:39
@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #495 (dc7f492) into master (fa602f5) will not change coverage.
The diff coverage is n/a.

❗ Current head dc7f492 differs from pull request most recent head 17c420e. Consider uploading reports for the commit 17c420e to get more accurate results

@@           Coverage Diff           @@
##           master     #495   +/-   ##
=======================================
  Coverage   60.37%   60.37%           
=======================================
  Files          45       45           
  Lines         795      795           
  Branches      146      146           
=======================================
  Hits          480      480           
  Misses        305      305           
  Partials       10       10           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa602f5...17c420e. Read the comment docs.

@davidjoy
Copy link
Contributor

davidjoy commented Jun 3, 2022

Hmm, can/should these PRs also remove es-check? Is there are reason not to?

@Jawayria
Copy link
Contributor

Jawayria commented Jun 6, 2022

Hmm, can/should these PRs also remove es-check? Is there are reason not to?

@adamstankiewicz might have a better idea about it.

@adamstankiewicz
Copy link
Member Author

@Jawayria @davidjoy Let's keep es-check, but check for ES6 rather than ES5. Validating against ES6 will help keep some guardrails in place to let us know when we use the latest JS syntax features, but our Babel configuration doesn't transpile it down to ES6 as expected.

I think it was a bit premature to assume we'd remove it earlier when I opened these PRs 😄

@Jawayria
Copy link
Contributor

Jawayria commented Jun 9, 2022

@Jawayria @davidjoy Let's keep es-check, but check for ES6 rather than ES5. Validating against ES6 will help keep some guardrails in place to let us know when we use the latest JS syntax features, but our Babel configuration doesn't transpile it down to ES6 as expected.

I think it was a bit premature to assume we'd remove it earlier when I opened these PRs 😄

@adamstankiewicz es6 checks fail on browserslist-config PRs but these tests are passing if a new PR is created against master branch.

@jsnwesson
Copy link
Contributor

Browserslist-config was added in a more recent PR. #649

@jsnwesson jsnwesson closed this Feb 24, 2023
@jsnwesson jsnwesson deleted the astankiewicz/shared-browserslist-config branch February 24, 2023 21:11
snglth pushed a commit to Abstract-Tech/community-theme-profile that referenced this pull request Jul 12, 2023
REV-2122: As part of the Value Prop implementation, we are removing the course sock from Course Home
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.

6 participants