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

remove unused deasync dependency #37821

Merged
merged 1 commit into from Apr 24, 2023
Merged

remove unused deasync dependency #37821

merged 1 commit into from Apr 24, 2023

Conversation

lsinger
Copy link
Contributor

@lsinger lsinger commented Apr 19, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Removes the deasync dependency, which seems to not be in use anymore. The package was causing a fatal build error during pnpm install when run from a path that contains spaces.

Closes #31882.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. clone trunk into a path with spaces, run nvm use and pnpm install and notice how it fails
  2. clone this branch into a path with spaces, run nvm use and pnpm install and notice how it does not fail
  3. ensure the package really isn't used anymore -- I wasn't able to find any actual code reference to the string deasync, and since that is how one uses the package I assume the package isn't used anymore. please double-check this assumption.

@lsinger lsinger added type: task The issue is an internally driven task (e.g. from another A8c team). plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Apr 19, 2023
@lsinger lsinger self-assigned this Apr 19, 2023
@lsinger lsinger requested review from peterfabian, a team and samueljseay and removed request for a team April 19, 2023 09:15
@github-actions
Copy link
Contributor

Hi @samueljseay, @peterfabian,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

Test Results Summary

Commit SHA: b469332

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 49s
E2E Tests1870010019714m 53s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

Copy link
Contributor

@samueljseay samueljseay left a comment

Choose a reason for hiding this comment

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

Nice find! Thanks for fixing this. I double checked this dep is certainly not used anywhere.

@codecov
Copy link

codecov bot commented Apr 24, 2023

Codecov Report

Merging #37821 (b469332) into trunk (e8fcfbb) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             trunk   #37821     +/-   ##
==========================================
- Coverage     51.5%    51.5%   -0.0%     
- Complexity   17261    17280     +19     
==========================================
  Files          429      430      +1     
  Lines        79957    80026     +69     
==========================================
+ Hits         41215    41216      +1     
- Misses       38742    38810     +68     
Impacted Files Coverage Δ
...ocommerce/includes/admin/class-wc-admin-assets.php 0.0% <ø> (ø)
...eta-boxes/class-wc-meta-box-product-categories.php 0.0% <0.0%> (ø)
plugins/woocommerce/includes/class-wc-ajax.php 4.3% <0.0%> (-0.1%) ⬇️

... and 1 file with indirect coverage changes

@lsinger lsinger merged commit 0fd5224 into trunk Apr 24, 2023
19 of 21 checks passed
@lsinger lsinger deleted the dev/remove-deasync-dep branch April 24, 2023 07:37
@github-actions github-actions bot added this to the 7.8.0 milestone Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin. type: task The issue is an internally driven task (e.g. from another A8c team).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error during build if there's a space in path
2 participants