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

feat: replace node-fetch by fetch default API #27

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

PaulGOUX27
Copy link
Collaborator

No description provided.

@PaulGOUX27 PaulGOUX27 requested a review from simonsmith June 25, 2024 16:02
@@ -85,7 +85,6 @@
"husky": "^4.3.0",
"import-sort-cli": "^6.0.0",
"import-sort-parser-typescript": "^6.0.0",
"import-sort-style-leboncoin": "^1.0.3",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got 404 on yarn install. Just removed it as the repo is not under development

@PaulGOUX27 PaulGOUX27 force-pushed the update-node-fetch-to-latest branch from 9d2ecbe to ba19d66 Compare June 26, 2024 07:13
@PaulGOUX27
Copy link
Collaborator Author

Put PR into draft as the node-fetch is an ESM only package starting 3.0.0-beta.10. I started to switch to fetch API included in node, but there is a bit of code to be added / tested. I will do it when I'm back

@PaulGOUX27 PaulGOUX27 marked this pull request as draft June 27, 2024 15:57
@PaulGOUX27 PaulGOUX27 force-pushed the update-node-fetch-to-latest branch 5 times, most recently from 55c3bc3 to 67145c8 Compare July 3, 2024 14:11
@PaulGOUX27 PaulGOUX27 marked this pull request as ready for review July 3, 2024 14:19
@PaulGOUX27
Copy link
Collaborator Author

@simonsmith I finished the PR. I removed a few things to make it run smoothly and fixed the release action by updating semantic-release-action to version 4. Merging it should create the new version.

@simonsmith
Copy link
Member

Amazing, let's try it

@simonsmith
Copy link
Member

simonsmith commented Jul 3, 2024

Oh one question, usually chore does not create a new release. Would we want to have a feat or fix here?

https://github.com/pvdlg/conventional-changelog-metahub#commit-types

Chores: Other changes that don't modify src or test files

@PaulGOUX27 PaulGOUX27 force-pushed the update-node-fetch-to-latest branch from 67145c8 to a7b0338 Compare July 4, 2024 07:26
@PaulGOUX27 PaulGOUX27 changed the title chore: update node-fetch to latest feat: update node-fetch to latest Jul 4, 2024
@PaulGOUX27
Copy link
Collaborator Author

I changed to feat, but I changed the version in the package.json to the next major (as the minimal node version was updated). But on your link there is no major ... Hope it will work 🤞

@PaulGOUX27 PaulGOUX27 changed the title feat: update node-fetch to latest feat: replace node-fetch by fetch default API Jul 4, 2024
@simonsmith
Copy link
Member

simonsmith commented Jul 4, 2024

You should be able to revert the version change and semantic release will do it for us. So the commit would have something like this in the description:

feat: do the change

BREAKING CHANGE: update node to whatever

That will trigger a 2.0.0 release

The breaking change part is detailed here for reference - https://www.conventionalcommits.org/en/v1.0.0/

BREAKING CHANGE: requires node >= 18
@PaulGOUX27 PaulGOUX27 force-pushed the update-node-fetch-to-latest branch from a7b0338 to 5c42f14 Compare July 4, 2024 12:10
@PaulGOUX27
Copy link
Collaborator Author

Alright, should be fine now. Thanks for sharing the convention, I didn't knew it exactly. Should we merge it now ?

@simonsmith simonsmith merged commit 1f8b942 into master Jul 4, 2024
3 checks passed
@simonsmith simonsmith deleted the update-node-fetch-to-latest branch July 4, 2024 12:37
Copy link

github-actions bot commented Jul 4, 2024

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants