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

refactor: use requestType instead of isAdmin, optionalIncludes #4115

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

thomasheartman
Copy link
Contributor

This removes the burden of knowing what optionalIncludes is and also
prevents weird edge cases like isAdmin with optionalIncludes.

This removes the burden of knowing what optionalIncludes is and also
prevents weird edge cases like isAdmin with optionalIncludes.
@vercel
Copy link

vercel bot commented Jun 29, 2023

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

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Jun 29, 2023 8:41am
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Jun 29, 2023 8:41am

Copy link
Contributor

@kwasniew kwasniew left a comment

Choose a reason for hiding this comment

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

It's beautiful :) I'd double check if the client query works fine (from the SDK) since it's the most impactful part. Looking at the code nothing should break but with client API facing stuff I'm always very careful.

@thomasheartman
Copy link
Contributor Author

Had a feeling you'd like this one 😄 And yeah, good point! I'll double check!

@thomasheartman
Copy link
Contributor Author

The Rust SDK (which we know is a little finicky) runs fine with these changes, as does the Go SDK, so I'm happy to merge this.

@thomasheartman thomasheartman merged commit 451c67a into main Jul 5, 2023
12 checks passed
@thomasheartman thomasheartman deleted the refactor/getall-request-type branch July 5, 2023 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants