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

Remove length limit from user id #202

Merged
merged 4 commits into from
Jan 9, 2025
Merged

Remove length limit from user id #202

merged 4 commits into from
Jan 9, 2025

Conversation

parfeon
Copy link
Contributor

@parfeon parfeon commented Jan 9, 2025

fix(user_id): remove length limit from user id

Remove the user_id length restriction for dynamically allocated memory for it.

build(CMakeLists): add try_compile configuration

Configure try_compile when building a static library.

Remove the `user_id` length restriction for dynamically allocated memory for it.

build(CMakeLists): add `try_compile` configuration

Configure `try_compile` when building static library.
@parfeon parfeon added status: done This issue is considered resolved. priority: high This PR should be reviewed ASAP. type: fix This PR contains fixes to existing features. labels Jan 9, 2025
@parfeon parfeon self-assigned this Jan 9, 2025
CMakeLists.txt Outdated
Comment on lines 63 to 71
num_option(USE_RETRY_CONFIGURATION "Use requests retry" OFF)
num_option(USE_REDidONFIGURATION "Use requests retry" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a typo? Below "USE_RETRY_CONFIGURATION" is still used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooh, don't wake computer with keyboard when last opened was your IDE :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@Xavrax Xavrax left a comment

Choose a reason for hiding this comment

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

LGTM however question:

  • Is server okay with this change. I though that this limitation has a reason.

@parfeon
Copy link
Contributor Author

parfeon commented Jan 9, 2025

@Xavrax Server code doesn't have any hard limits on uuid size, and other SDKs use pretty long identifiers without any issues.

@parfeon
Copy link
Contributor Author

parfeon commented Jan 9, 2025

@pubnub-release-bot release

@parfeon parfeon merged commit 11fc03a into master Jan 9, 2025
5 checks passed
@parfeon parfeon deleted the CLEN-2461 branch January 9, 2025 10:06
@pubnub-release-bot
Copy link
Contributor

🚀 Release successfully completed 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high This PR should be reviewed ASAP. status: done This issue is considered resolved. type: fix This PR contains fixes to existing features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants