-
Notifications
You must be signed in to change notification settings - Fork 23
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
GitHub Porter URIs #555
GitHub Porter URIs #555
Conversation
5bfe9bf
to
59a0df1
Compare
@piotr-roslaniec @cygnusv @theref To finish that PR I'd like to ask you where JSON should be stored? It can be empty for the beginning |
59a0df1
to
742757c
Compare
IMHO https://github.com/nucypher/nucypher-porter repo is fine for this purpose |
a6cf462
to
8961a5a
Compare
✅ Deploy Preview for taco-nft-demo canceled.
|
✅ Deploy Preview for taco-demo canceled.
|
packages/shared/src/porter.ts
Outdated
protected async tryAndCall<T, D>( | ||
config: AxiosRequestConfig<D>, | ||
): Promise<AxiosResponse<T>> { | ||
let resp!: AxiosResponse<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the !
assertion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to mark it's 100% assigned, vs code was showing error without it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a minor smell so I guess we let it slide. But at any rate, let
statements where we assign value later should not be required to !
if the branching is done properly
@vzotova Before we merge this, it may be helpful to run the Node.js example using these new Porter URLs |
22ff9ac
to
346190d
Compare
Co-authored-by: piotr-roslaniec <39299780+piotr-roslaniec@users.noreply.github.com>
346190d
to
565eed4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #555 +/- ##
===========================================
+ Coverage 23.12% 79.56% +56.43%
===========================================
Files 62 65 +3
Lines 10175 6806 -3369
Branches 260 299 +39
===========================================
+ Hits 2353 5415 +3062
+ Misses 7763 1349 -6414
+ Partials 59 42 -17 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎸
Type of PR:
Required reviews:
What this does:
Issues fixed/closed:
Fixes #320
Why it's needed:
Notes for reviewers:
Based on #553