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

docs: clarify legacy deps #2184

Merged
merged 10 commits into from
Dec 20, 2023
Merged

docs: clarify legacy deps #2184

merged 10 commits into from
Dec 20, 2023

Conversation

Zamiell
Copy link
Contributor

@Zamiell Zamiell commented Dec 18, 2023

Since isUtf8 is automatically present in modern versions of Node.js, we can cleanup this old info that doesn't apply anymore in order to simplify things and make the docs easier to read.

(It's also a footgun for people reading this passage and then installing both bufferutil and utf-8-validate.)

@lpinca
Copy link
Member

lpinca commented Dec 19, 2023

buffer.isUtf8() is only available in Node.js >= 18.14.0. We still support Node.js 10, so this is not acceptable.

@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 19, 2023

Sorry about that. In order to satisfy the requirements, I have updated this PR to delineate the information into two sections, one of which is clearly for legacy consumers of the library.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lpinca
Copy link
Member

lpinca commented Dec 19, 2023

Can you also please use a more descriptive commit message?

@Zamiell Zamiell changed the title docs: update README.md docs: clarify legacy deps Dec 19, 2023
Zamiell and others added 2 commits December 19, 2023 13:36
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 19, 2023

i assume by commit message you mean PR title, so I changed that now

@lpinca
Copy link
Member

lpinca commented Dec 19, 2023

No, I meant the commit message title, but I can change it before merging.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lpinca
Copy link
Member

lpinca commented Dec 19, 2023

Please fix the formatting issues.

Zamiell and others added 3 commits December 19, 2023 22:54
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@Zamiell
Copy link
Contributor Author

Zamiell commented Dec 20, 2023

done

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lpinca lpinca merged commit d37756a into websockets:master Dec 20, 2023
56 checks passed
@lpinca
Copy link
Member

lpinca commented Dec 20, 2023

Thank you.

@Zamiell Zamiell deleted the patch-1 branch December 20, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants