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

Add user agent header #111

Merged
merged 3 commits into from
Dec 29, 2023
Merged

Conversation

VishalMCF
Copy link
Contributor

resolves #104

@VishalMCF
Copy link
Contributor Author

@Cliftonz Can you please review?

@unicodeveloper
Copy link
Contributor

@mayorJAY Please review

@Cliftonz Cliftonz merged commit 5abd545 into novuhq:main Dec 29, 2023
1 check passed
@mayorJAY
Copy link
Collaborator

mayorJAY commented Dec 29, 2023

This implementation would give us the name and version of the client app instead of the info of this SDK. We need to revert and revisit the implementation @Cliftonz @unicodeveloper @VishalMCF

@VishalMCF
Copy link
Contributor Author

VishalMCF commented Dec 29, 2023

@mayorJAY @Cliftonz @unicodeveloper I was thinking of an approach where I hardcode the version in the RetryHandlerClass. Name as you said can be hardcoded. I can write a test function that always checks if the version is the same as what we have in the pom.xml file. What do you say? That way don't have to risk reading the file during runtime.

@mayorJAY
Copy link
Collaborator

@mayorJAY @Cliftonz @unicodeveloper I was thinking of an approach where I hardcode the version in the RetryHandlerClass. Name as you said can be hardcoded. I can write a test function that always checks if the version is the same as what we have in the pom.xml file. What do you say? That way don't have to risk reading the file during runtime.

I don't think we should hardcode the version, that would mean updating the header every time there is a version change. If we modify the code based on my suggestions, we'd get what we need

@VishalMCF
Copy link
Contributor Author

@mayorJAY cool, I will raise a new PR as per your suggestions

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.

Add User Agent Header
4 participants