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.

Sign up for GitHub

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

Jump to bottom

handle remote param authentication #258

Merged
merged 2 commits into from
Nov 4, 2022
Merged

handle remote param authentication #258

merged 2 commits into from
Nov 4, 2022

Conversation

kdai7
Copy link
Contributor

@kdai7 kdai7 commented Nov 4, 2022

No description provided.

@kdai7 kdai7 requested a review from carrolp November 4, 2022 19:34
carrolp
carrolp previously approved these changes Nov 4, 2022
View reviewed changes
adamkingit
adamkingit previously approved these changes Nov 4, 2022
View reviewed changes
Copy link
Contributor

@adamkingit adamkingit left a comment

Choose a reason for hiding this comment

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

Consider using the base object, then adding the headers property if auth is set so updates to the invariant properties don't have to be made 4 times whenever there is an update. If after consideration you prefer the readability as is, I approve.

@kdai7 kdai7 dismissed stale reviews from adamkingit and carrolp via a4fa994 November 4, 2022 20:01
@kdai7 kdai7 merged commit 45bbc45 into master Nov 4, 2022
@kdai7 kdai7 deleted the auth branch November 4, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carrolp carrolp carrolp approved these changes

@adamkingit adamkingit Awaiting requested review from adamkingit

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
@kdai7 @adamkingit @carrolp