Skip to content

Conversation

guptamukund22
Copy link

  • Refactored "OauthWeblnProvider.ts" by implementing "execute" method for better readability and modularity.

@guptamukund22 guptamukund22 changed the title Modularize the code Refactor : Modularize the code Apr 3, 2025
@guptamukund22 guptamukund22 changed the title Refactor : Modularize the code Refactor : enhanced readability of the code Apr 4, 2025
@bumi
Copy link
Contributor

bumi commented Apr 6, 2025

#344 moves some code around. once that one is merged maybe you can update this one again and we'll review it. thanks

@rolznz
Copy link
Contributor

rolznz commented Apr 14, 2025

Hi @guptamukund22 could you please make sure to branch off master before making each change, so you don't have changes from the previous PR in this one too.

Sorry for the conflicts - we just merged the big change Bumi mentioned above.

@pavanjoshi914
Copy link
Contributor

@guptamukund22 can you please try to resolve the conflicts and then i can review it again thanks!

request_options?: Partial<RequestOptions>,
): Promise<GetAccountBalanceResponse> {
// HTTP handlers
private httpGet<T>(
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of having httpGet httpPost httpDelete methods. can we have generic http Request method here?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, Actually I have my end semester exams going on that's why I was not able to reply. I will fix the issue asap.

accountInformation(
// eslint-disable-next-line @typescript-eslint/ban-types
params: {},
request_options?: Partial<RequestOptions>,
Copy link
Contributor

Choose a reason for hiding this comment

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

why params and request_options from all the methods were removed. this will break things for package users. we should refactor but not change current working of library

@rolznz rolznz marked this pull request as draft May 29, 2025 08:14
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