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

Convert Routes to use Constants #40

Open
saivineeth100 opened this issue Jun 25, 2024 · 5 comments
Open

Convert Routes to use Constants #40

saivineeth100 opened this issue Jun 25, 2024 · 5 comments

Comments

@saivineeth100
Copy link

saivineeth100 commented Jun 25, 2024

Existing

private readonly Dictionary<string, string> _routes = new Dictionary<string, string>
{
    ["parameters"] = "/parameters",
    
}

Suggested
Add to Constants.cs or Add new File Routes.cs

    public static class Routes
    {
        public const string Parameters = "/parameters";
        public static class Auth
        {
            public const string Token = "/session/token";
            public const string RefreshToken = "/session/refresh_token";
        }

    }

When using we can use

Post(Routes.Auth.Token, reqParams)
@ajinasokan
Copy link
Member

Thanks for the suggestion. But this will be quite a significant change. Will consider this for the next major version. I will keep the issue open.

@saivineeth100
Copy link
Author

Thanks for the suggestion. But this will be quite a significant change. Will consider this for the next major version. I will keep the issue open.

Can I contribute to this or any suggestions via pull requests?

@ajinasokan
Copy link
Member

I actually have the patch ready. But just not merged yet. Since this is changing code related to all APIs, it will have to be tested as a whole and that is time consuming. That is why I'm delaying it till a major version or a refactor.

@saivineeth100
Copy link
Author

I am asking in general, can outsiders contribute to Zerodha repos?

@ajinasokan
Copy link
Member

Yeah sure. Just keep in mind, if it is a large patch then make sure to create an issue with your suggestion and seek maintainers opinion.

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

No branches or pull requests

2 participants