Skip to content
This repository has been archived by the owner on Mar 10, 2024. It is now read-only.

feat: @supaglue/sdk first release #2033

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: @supaglue/sdk first release #2033

wants to merge 6 commits into from

Conversation

tonyxiao
Copy link
Contributor

@tonyxiao tonyxiao commented Dec 6, 2023 •
edited by lucasmarshall

No description provided.

@vercel Vercel
Copy link

vercel bot commented Dec 6, 2023 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
management-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 6, 2023 11:53pm
supaglue-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 6, 2023 11:53pm

@tonyxiao
Copy link
Contributor Author

tonyxiao commented Dec 6, 2023

I'm thinking of changing @supaglue/schemas to simply @supaglue/sdk, it's not technically 100% correct naming given it contains a bunch of unrelated code but feels like the least friction overall and not much tech debt being introduced.

@tonyxiao tonyxiao changed the title Tony/supaglue sdk feat: @supaglue/sdk first release Dec 6, 2023
@tonyxiao tonyxiao marked this pull request as ready for review December 6, 2023 23:40
@tonyxiao
Copy link
Contributor Author

tonyxiao commented Dec 6, 2023

https://www.npmjs.com/package/@supaglue/sdk?activeTab=readme

Need to add docs, but already works!

asdfryan
asdfryan reviewed Dec 7, 2023
View reviewed changes
Copy link
Contributor

@asdfryan asdfryan left a comment

Choose a reason for hiding this comment

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

How much effort would it be to migrate our existing integration tests to use the SDK?

@tonyxiao
Copy link
Contributor Author

tonyxiao commented Dec 7, 2023

Not much. We already do in at least one integration test. Should be a straightforward refactor.

lucasmarshall
lucasmarshall suggested changes Dec 8, 2023
View reviewed changes
packages/sdk/package.json
@@ -1,9 +1,11 @@
{
"name": "@supaglue/schemas",
"name": "@supaglue/sdk",
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we tested this in a non-monorepo project? We should have to set main, types or exports at least so that we can specify where the index.js file is.

Suggested change
"name": "@supaglue/sdk",
"name": "@supaglue/sdk",
"publishConfig": {
"main": "./dist/index.js",
"types": "./dist/index.d.ts"
},

Should do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, I tested it via grabbing the published npm package and confirm it works.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@asdfryan asdfryan asdfryan left review comments

@lucasmarshall lucasmarshall lucasmarshall requested changes

@tomkit tomkit Awaiting requested review from tomkit tomkit is a code owner

@george-xing george-xing Awaiting requested review from george-xing george-xing is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tonyxiao @lucasmarshall @asdfryan