Skip to content

Remove cross-fetch dependency#1688

Merged
meili-bors[bot] merged 2 commits intomeilisearch:mainfrom
flevi29:remove-cross-fetch
Sep 16, 2024
Merged

Remove cross-fetch dependency#1688
meili-bors[bot] merged 2 commits intomeilisearch:mainfrom
flevi29:remove-cross-fetch

Conversation

@flevi29
Copy link
Copy Markdown
Collaborator

@flevi29 flevi29 commented Aug 15, 2024

Pull Request

Related issue

Fixes #1658

What does this PR do?

  • removes cross-fetch dependency
  • if anyone might need the polyfill they should run it themselves
  • there's an interesting discussion here about polyfills in libraries; I think, as a lot of people do, that the sane and clean path ahead is to make it the responsibility of the library user to polyfill, even if it's somewhat of a burden

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@flevi29 flevi29 added the breaking-change The related changes are breaking for the users label Aug 15, 2024
@flevi29 flevi29 requested a review from brunoocasali August 15, 2024 08:56
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.11%. Comparing base (bb4f53e) to head (ee63ac8).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1688      +/-   ##
==========================================
+ Coverage   96.99%   97.11%   +0.11%     
==========================================
  Files          21       21              
  Lines         833      831       -2     
  Branches       85       77       -8     
==========================================
- Hits          808      807       -1     
+ Misses         25       23       -2     
- Partials        0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

LGTM!

bors merge

meili-bors Bot added a commit that referenced this pull request Sep 15, 2024
1688: Remove cross-fetch dependency r=brunoocasali a=flevi29

# Pull Request

## Related issue
Fixes #1658 

## What does this PR do?
- removes `cross-fetch` dependency
- if anyone might need the polyfill they should run it themselves
- there's an interesting discussion [here](w3ctag/polyfills#6 (comment)) about polyfills in libraries; I think, as a lot of people do, that the sane and clean path ahead is to make it the responsibility of the library user to polyfill, even if it's somewhat of a burden

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


1698: Add `"packageManager"` entry to package.json r=brunoocasali a=flevi29

# Pull Request

## Why?
- [`corepack`](https://nodejs.org/docs/latest-v20.x/api/corepack.html) [started adding this field automatically](nodejs/corepack#413), which can be disabled, but everyone has to do it separately, which is very annoying
- anyhow there's no harm in it, `corepack` recommends it even in the "how to disable" doc https://github.com/nodejs/corepack/blob/main/README.md#environment-variables
  - > it ensures that your project installs are always deterministic (if you use `corepack`)
- this does mean that this field will have to be updated periodically, but this is the recommended way

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: F. Levi <55688616+flevi29@users.noreply.github.com>
@meili-bors
Copy link
Copy Markdown
Contributor

meili-bors Bot commented Sep 15, 2024

This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried.

Additional information:

{"message":"Changes must be made through a pull request.","documentation_url":"https://docs.github.com/articles/about-protected-branches","status":"422"}

@brunoocasali
Copy link
Copy Markdown
Member

bors merge

@meili-bors
Copy link
Copy Markdown
Contributor

meili-bors Bot commented Sep 16, 2024

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change The related changes are breaking for the users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove cross-fetch dependency UMD bundled version theoretically should err on browsers without native fetch support

2 participants