Skip to content

Add access token support#19

Merged
petevb merged 3 commits intoNewOrbit:masterfrom
kamilturek:access-token
Jul 18, 2022
Merged

Add access token support#19
petevb merged 3 commits intoNewOrbit:masterfrom
kamilturek:access-token

Conversation

@kamilturek
Copy link
Copy Markdown
Contributor

@kamilturek kamilturek commented Jul 11, 2022

@kamilturek kamilturek marked this pull request as ready for review July 12, 2022 15:14
@kamilturek
Copy link
Copy Markdown
Contributor Author

kamilturek commented Jul 15, 2022

Hey @OwenPattison, any chance of getting this reviewed? Many thanks!

@petevb petevb self-assigned this Jul 15, 2022
@petevb
Copy link
Copy Markdown
Contributor

petevb commented Jul 15, 2022

@kamilturek only just seen this sorry. Will pick it up on Monday. Thanks for the PR!

petevb
petevb reviewed Jul 18, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@petevb petevb left a comment

Choose a reason for hiding this comment

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

Tiny nit, otherwise LGTM. Thanks!

Comment thread index.ts
petevb
petevb approved these changes Jul 18, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@petevb petevb left a comment

Choose a reason for hiding this comment

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

:shipit:

@petevb
Copy link
Copy Markdown
Contributor

petevb commented Jul 18, 2022

linter didn't like that - reverting.

petevb
petevb reviewed Jul 18, 2022
View reviewed changes
Comment thread index.ts Outdated
@petevb petevb merged commit 77e7909 into NewOrbit:master Jul 18, 2022
@kamilturek kamilturek deleted the access-token branch July 19, 2022 20:06
@kamilturek
Copy link
Copy Markdown
Contributor Author

kamilturek commented Jul 19, 2022 •
edited
Loading

@petevb Thanks for the review. I think that the reason your suggestion didn't work is that the optional chaining was introduced in TypeScript 3.7 and this package uses 3.0.3.

targetprocess-rest-api/package.json

Line 40 in fe51913

"typescript": "^3.0.3"

I didn't want to bump up the version on my own though.

@petevb
Copy link
Copy Markdown
Contributor

petevb commented Jul 20, 2022

@petevb Thanks for the review. I think that the reason your suggestion didn't work is that the optional chaining was introduced in TypeScript 3.7 and this package uses 3.0.3.

targetprocess-rest-api/package.json

Line 40 in fe51913

"typescript": "^3.0.3"

I didn't want to bump up the version on my own though.

Makes sense, thank you for coming back to me and pointing that out - I assumed it was something like that but didn't have time to investigate. Thanks again for the PR, @kamilturek! 🥇

@petevb petevb removed their assignment Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@petevb petevb petevb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kamilturek @petevb