Skip to content

Conversation

@danielailie
Copy link
Contributor

No description provided.

@danielailie danielailie self-assigned this Oct 14, 2024
popenta
popenta previously approved these changes Oct 14, 2024
README.md Outdated

### axios

This package can make HTTP requests using `axios`, but it is not bundled by default. If you plan to use the API network provider or Proxy network provider, make sure to install `axios`:
Copy link
Contributor

Choose a reason for hiding this comment

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

but it is not -> which is not (optional) (here and below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

private async doGet(resourceUrl: string): Promise<any> {
let axios = await getAxios();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be const (here and below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

src/utils.ts Outdated
export async function getAxios() {
let axios;
try {
axios = require("axios");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can return directly, without declaring let axios above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


export class BLS {
private static isInitialized: boolean = false;
public static bls: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit tricky. C#'s internal would have been nice here :)

Though, since we plan to drop Herumi's library, it's fine 👍

Alternative: private field and a public function getUnderlyingComponent() (or something similar).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will let it like this since we plan to drop it

@danielailie danielailie merged commit fe5f1b4 into feat/unify Oct 15, 2024
4 checks passed
@danielailie danielailie deleted the TOOL-264-bring-sdk-wallet-into-sdk-core-optional-dependencies branch October 15, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants