Skip to content
This repository was archived by the owner on Nov 15, 2024. It is now read-only.

Improvements to client session methods #191

Merged
razor-x merged 6 commits into
mainfrom
cleanup-get-client-session
May 2, 2023
Merged

Improvements to client session methods #191
razor-x merged 6 commits into
mainfrom
cleanup-get-client-session

Conversation

@razor-x

@razor-x razor-x commented Apr 27, 2023

Copy link
Copy Markdown
Member

@razor-x razor-x force-pushed the cleanup-get-client-session branch from 69c92b6 to fc21ce2 Compare May 2, 2023 04:45
@razor-x razor-x force-pushed the cleanup-get-client-session branch from fc21ce2 to 8a26dd7 Compare May 2, 2023 04:52
@razor-x razor-x marked this pull request as ready for review May 2, 2023 04:57
@razor-x razor-x requested a review from codetheweb as a code owner May 2, 2023 04:57
@razor-x razor-x requested a review from seveibar May 2, 2023 04:57
@razor-x razor-x changed the title Reuse makeRequest Improvements to client session methods May 2, 2023
getSeamClientOptionsWithDefaults(ops)
let headers: AxiosRequestHeaders = {
const headers = {
"seam-user-identifier-key": options.userIdentifierKey,

@seveibar seveibar May 2, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not opposed to this header, but i think i lean towards this being preferred in the payload, allowed in header/. headers distract from the concept that we're basically an RPC API, we only use them where convention or security pushes us to. That being said, I think it's ok that it's accepted as a header.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this is the existing implementation (not added in this PR). We can make the switch but will need to check the endpoint first to see if it will accept this in the body.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yep that's fine!!!

@razor-x razor-x merged commit 5b0696d into main May 2, 2023
@razor-x razor-x deleted the cleanup-get-client-session branch May 2, 2023 17:44
@seambot

seambot commented May 2, 2023

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 5.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants