Skip to content

Switch to node-apis, hook up UserClass, fix tests#37

Merged
andrew-propelauth merged 4 commits intomainfrom
switch-to-node-apis
May 10, 2024
Merged

Switch to node-apis, hook up UserClass, fix tests#37
andrew-propelauth merged 4 commits intomainfrom
switch-to-node-apis

Conversation

@andrew-propelauth
Copy link
Copy Markdown
Contributor

Move shared library and types into the node-apis repo.

This switches to fetch which should work with more environments.
Also hooks up UserClass and adds a new validation function for it.
Then fixes the tests which now need fetch

mrmauer
mrmauer previously requested changes Apr 8, 2024
Copy link
Copy Markdown
Contributor

@mrmauer mrmauer left a comment

Choose a reason for hiding this comment

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

Broadly LGTM, just a few things:

  1. There's a few places we need to make sure not to let the local yalc dependency sneak into our published source code.
  2. We should do a gut check that the UserClass from node-apis supports all the same methods (same names) as the UserClass previously defined here. That might just mean more coverage on the unit tests here in a way that may be redundant with tests in node-apis?

@itailevi98 itailevi98 force-pushed the switch-to-node-apis branch from c69ba9a to e871641 Compare May 8, 2024 17:19
@itailevi98 itailevi98 self-assigned this May 8, 2024
@itailevi98 itailevi98 requested review from mrmauer and removed request for itailevi98 May 8, 2024 17:20
@andrew-propelauth
Copy link
Copy Markdown
Contributor Author

lgtm w/ the new changes

@andrew-propelauth andrew-propelauth merged commit 79eb7f5 into main May 10, 2024
@andrew-propelauth andrew-propelauth deleted the switch-to-node-apis branch May 10, 2024 19:08
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.

3 participants