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.

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

refactor: migrate Client base methods to TypeScript #1153

Merged
merged 7 commits into from May 21, 2023

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented May 17, 2023

No description provided.

@trim21 trim21 marked this pull request as ready for review May 17, 2023 13:54
@trim21 trim21 changed the title refactor: migrate base Client methods to TypeScript refactor: migrate Client base methods to TypeScript May 18, 2023
prakashsvmx
prakashsvmx previously approved these changes May 18, 2023
Copy link
Member

@prakashsvmx prakashsvmx left a comment

Choose a reason for hiding this comment

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

Tested LGTM 👍🎉

src/internal/client.ts Outdated Show resolved Hide resolved
@prakashsvmx
Copy link
Member

One more suggestion on the ts compiler option we can adopt in later/different pr is:
"strictPropertyInitialization": false,

as strict init is not possible always for properties. please share your thoughts as well around this.

@trim21
Copy link
Contributor Author

trim21 commented May 19, 2023

One more suggestion on the ts compiler option we can adopt in later/different pr is: "strictPropertyInitialization": false,

as strict init is not possible always for properties. please share your thoughts as well around this.

There is no need to disable this option I think, at least in minio-js.

If a property is optional (like CredentialsProvider with setCredentialsProvider and ClientOptions), it need to used as nullable value.

I can't think of a property in this project that they need to disable this option

@harshavardhana harshavardhana merged commit 5144d0d into minio:master May 21, 2023
14 checks passed
@trim21 trim21 deleted the base-ts-client branch May 21, 2023 03:25
trim21 added a commit to trim21/minio-js that referenced this pull request May 24, 2023
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.

None yet

3 participants