Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Switch to Scala.js dependency for saslprep #587

Merged
merged 4 commits into from
Dec 13, 2021

Conversation

armanbilge
Copy link
Member

This replaces the npm dependency for saslprep with a pure Scala.js implementation. This lets us to dump npm, dump ScalaJSBundler, dump the 8000 lines of lockfiles, and all the associated maintenance burden.

The Scala saslprep implementation is just a port from MongoDB. I originally started adding this code directly to skunk, but I ended microlib-ing it instead because it seemed to be getting out-of-scope. It's a single file and test suite so happy to just include it in skunk if you don't mind the bloat (i.e. a stupid number of hardcoded UTF8 codepoints. Frustrating that Node.js doesn't have this stuff out-of-the-box).

@armanbilge armanbilge marked this pull request as draft December 11, 2021 15:34
@codecov-commenter
Copy link

codecov-commenter commented Dec 11, 2021

Codecov Report

Merging #587 (b911028) into main (83b7e2c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #587   +/-   ##
=======================================
  Coverage   85.30%   85.30%           
=======================================
  Files         124      124           
  Lines        1599     1599           
  Branches       34       34           
=======================================
  Hits         1364     1364           
  Misses        235      235           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83b7e2c...b911028. Read the comment docs.

@armanbilge armanbilge marked this pull request as ready for review December 11, 2021 16:47
@armanbilge armanbilge mentioned this pull request Dec 13, 2021
6 tasks
@tpolecat
Copy link
Member

BRILLIANT!

@tpolecat tpolecat merged commit 3c5f822 into typelevel:main Dec 13, 2021
@keynmol
Copy link
Contributor

keynmol commented Dec 13, 2021

This lets us to dump npm, dump ScalaJSBundler, dump the 8000 lines of lockfiles, and all the associated maintenance burden

Must say, this entire sentence made me all hot and bothered. Very arousing.

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

Successfully merging this pull request may close these issues.

4 participants