Replace node-fetch with centra#70
Replace node-fetch with centra#70maybeanerd merged 4 commits intobotblock:masterfrom MeerBiene:master
Conversation
| "lockfileVersion": 1, | ||
| "lockfileVersion": 2, |
There was a problem hiding this comment.
This might be considered a breaking change?
There was a problem hiding this comment.
IDK, is this breaking?
I mean as long as it doesn't change the way devs are supposed to use BLAPI, it can't really break anything, right?
But I'm not entirely sure if it still can't be considered breaking.
I'd definitely bump the minor version though
There was a problem hiding this comment.
Is that something yarn does? I don't usually use yarn and I don't want to produce a breaking change tbh
There was a problem hiding this comment.
@MattIPv4 what do you think? what's the main reason you would count it as a breaking change?
There was a problem hiding this comment.
pretty sure this will be annoying for anyone on node <15 trying to download/manage deps for editing the lib, but afaik it shouldn't affect end-users of the lib?
There was a problem hiding this comment.
npm 7 apparently also generates a lockfile with version 2 "[...] which is backwards compatible with npm 6 users [...]" https://github.blog/2021-02-02-npm-7-is-now-generally-available/
There was a problem hiding this comment.
npm 7 apparently also generates a lockfile with version 2 "[...] which is backwards compatible with npm 6 users [...]" https://github.blog/2021-02-02-npm-7-is-now-generally-available/
nice, thanks for that info!
To me, this looks like a non-breaking change then - for development having at least npm v6 is something we could assume imo. It's not like there are many devs besides me and new people who open PRs anyways 🤷🏻♂️
There was a problem hiding this comment.
You could maybe write a contributing file where you can put information about the dev environment
There was a problem hiding this comment.
sounds good.
would you push the version in package.json to 2.1.0? I think then I can merge it onto master.
And i'm assuming you tested if it works? we could rely solely on the typings, but a real test would still be good just in case.
There was a problem hiding this comment.
Will do.
Yes I've tested the botblock way and the manual posting. Also made sure botblocks ratelimits are working :D
maybeanerd
left a comment
maybeanerd
left a comment
There was a problem hiding this comment.
LGTM
after deciding on the matter of breaking change or not, I'd bump the version accordingly and merge.
4 participants
As mentioned in #69 (nice) I replaced node-fetch with centra and made sure the functionality isn't compromised.