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.

Sign up for GitHub

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

Jump to bottom

feat: Enable gzip support for event payloads #379

Merged
merged 1 commit into from
May 15, 2024
Merged

feat: Enable gzip support for event payloads #379

merged 1 commit into from
May 15, 2024

Conversation

keelerm84
Copy link
Member

No description provided.

@keelerm84 keelerm84 requested a review from a team May 6, 2024 17:25
@keelerm84 keelerm84 force-pushed the mk/sc-243105/event-gzip branch from c398357 to 18d4e06 Compare May 6, 2024 17:53
tanderson-ld
tanderson-ld approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@tanderson-ld tanderson-ld left a comment

Choose a reason for hiding this comment

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

Small comment, the parameter is called body which is generic, but then the internal variable is called eventPayload which is more specific. This indicates an assumption about how this function is being used. Consider not using as specific a name. Maybe, bodyToUse?

@keelerm84
Copy link
Member Author

Small comment, the parameter is called body which is generic, but then the internal variable is called eventPayload which is more specific. This indicates an assumption about how this function is being used. Consider not using as specific a name. Maybe, bodyToUse?

Done! I went with httpBody to match the property assignment.

@keelerm84 keelerm84 force-pushed the mk/sc-243105/event-gzip branch from a29a146 to 6921299 Compare May 15, 2024 13:50
keelerm84
keelerm84 commented May 15, 2024
View reviewed changes
LaunchDarkly.podspec
@@ -39,5 +39,8 @@ Pod::Spec.new do |ld|

ld.subspec 'Core' do |es|
es.dependency 'LDSwiftEventSource', '3.1.1'
es.ios.dependency 'GZIP', '1.3.2'
Copy link
Member Author

Choose a reason for hiding this comment

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

This gzip library doesn't support watchos. Which is probably reasonable since CPU cycles are going to be more precious on that device.

@keelerm84 keelerm84 requested a review from tanderson-ld May 15, 2024 13:52
@keelerm84 keelerm84 merged commit 8987305 into feat/gzip-events May 15, 2024
4 checks passed
@keelerm84 keelerm84 deleted the mk/sc-243105/event-gzip branch May 15, 2024 17:33
keelerm84 added a commit that referenced this pull request Aug 1, 2024
@keelerm84
feat: Enable gzip support for event payloads (#379)
6bb549a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tanderson-ld tanderson-ld tanderson-ld approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@keelerm84 @tanderson-ld