Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

build: use shared browserslist configuration #63

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

Mashal-m
Copy link
Contributor

@Mashal-m Mashal-m commented Sep 21, 2022

Ticket:
Use shared @edx/browserslist-config

Removes custom browserslist configuration in favor of using a Shared Configuration

This change reduces the resultant asset bundle size.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 21, 2022
@openedx-webhooks
Copy link

Thanks for the pull request, @Mashal-m! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

⚠️ We can't start reviewing your pull request until you've submitted a signed contributor agreement or indicated your institutional affiliation. Please see the CONTRIBUTING file for more information. If you've signed an agreement in the past, you may need to re-sign. See The New Home of the Open edX Codebase for details.

@coveralls
Copy link

coveralls commented Sep 21, 2022

Pull Request Test Coverage Report for Build 3489579138

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 54.95%

Totals Coverage Status
Change from base Build 3489041364: 0.0%
Covered Lines: 167
Relevant Lines: 307

💛 - Coveralls

@codecov
Copy link

codecov bot commented Sep 21, 2022

Codecov Report

Base: 57.28% // Head: 57.28% // No change to project coverage 👍

Coverage data is based on head (3b166c9) compared to base (d2c8115).
Patch has no changes to coverable lines.

❗ Current head 3b166c9 differs from pull request most recent head 55d997f. Consider uploading reports for the commit 55d997f to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #63   +/-   ##
=======================================
  Coverage   57.28%   57.28%           
=======================================
  Files          26       26           
  Lines         398      398           
  Branches       66       66           
=======================================
  Hits          228      228           
  Misses        154      154           
  Partials       16       16           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@natabene natabene changed the title build: use shared browserslist configuration build: use shared browserslist configuration Sep 27, 2022
@Mashal-m Mashal-m closed this Oct 4, 2022
@Mashal-m Mashal-m reopened this Oct 4, 2022
@natabene
Copy link

natabene commented Oct 5, 2022

@schenedx Ready for your team whenever.

@schenedx
Copy link
Contributor

schenedx commented Oct 5, 2022

@Mashal-m Why is this PR created? Why do we want to adopt using browser-list-config instead of what's already there? The description is empty and that would make it difficult for reviewer to understand the intent.

@Mashal-m
Copy link
Contributor Author

Mashal-m commented Oct 6, 2022

@schenedx This change reduces the resultant asset bundle size.
I have added the description

@ashultz0 ashultz0 force-pushed the mashal-m/add-browserslist-config branch from 3b166c9 to 55d997f Compare November 17, 2022 15:21
@ashultz0
Copy link
Contributor

@Mashal-m I had to make some updates to packages.json to get this deployable again and I have rebased your package.json changes on top of those, merging them in was very easy, and then rebuilt package locks

@ashultz0
Copy link
Contributor

failing test is just a failure to upload to codecov which has been intermittently annoying me over the past couple of days, I'm going to merge and recheck on stage

@ashultz0 ashultz0 merged commit 0810183 into master Nov 17, 2022
@ashultz0 ashultz0 deleted the mashal-m/add-browserslist-config branch November 17, 2022 15:34
@ashultz0
Copy link
Contributor

deployed, looks OK

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants