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: add culture context to event #2036

Merged
merged 8 commits into from
Aug 4, 2022
Merged

feat: add culture context to event #2036

merged 8 commits into from
Aug 4, 2022

Conversation

kevinrenskers
Copy link
Contributor

@kevinrenskers kevinrenskers commented Aug 2, 2022 •
edited
Loading

📜 Description

A picture is worth a 1000 words.

Screen Shot 2022-08-02 at 17 33 19

💡 Motivation and Context

Closes #1534.

💚 How did you test it?

Manually + unit tests

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

@github-actions
Copy link

github-actions bot commented Aug 2, 2022 •
edited
Loading

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against add327c

@kevinrenskers kevinrenskers changed the title feat: add locale context to event feat: add culture context to event Aug 3, 2022
@kevinrenskers kevinrenskers marked this pull request as ready for review August 3, 2022 08:14
armcknight
armcknight approved these changes Aug 3, 2022
View reviewed changes
Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

Changes look good.

I would personally call this "locale info" vs "culture context": "culture" and "locale" aren't interchangeable terms, and using the platform-standard terminology helps searchability/discovery.

@kevinrenskers
Copy link
Contributor Author

I just named it according to the docs: https://develop.sentry.dev/sdk/event-payloads/contexts/#culture-context

@kevinrenskers kevinrenskers merged commit 04619e5 into master Aug 4, 2022
@kevinrenskers kevinrenskers deleted the feat/1534-culture-context branch August 4, 2022 07:14
kevinrenskers added a commit that referenced this pull request Aug 4, 2022
@kevinrenskers
Merge branch 'master' into ref/1696-DRY-shouldBeDisabled
d0b78b8 
* master:
  feat: add culture context to event (#2036)
  feat: Report App Memory Usage (#2027)
  ci: run profile generator 4x per day (#2038)
  build(deps): bump actions/stale from 5.1.0 to 5.1.1 (#2030)

# Conflicts:
#	Sentry.xcodeproj/project.pbxproj
kevinrenskers added a commit that referenced this pull request Aug 4, 2022
@kevinrenskers
Merge branch 'master' into style/120-chars
349fc2b 
* master:
  feat: add culture context to event (#2036)
  feat: Report App Memory Usage (#2027)
  ci: run profile generator 4x per day (#2038)
  build(deps): bump actions/stale from 5.1.0 to 5.1.1 (#2030)

# Conflicts:
#	Sources/Sentry/SentryClient.m
@kevinrenskers kevinrenskers mentioned this pull request Aug 24, 2022
Closed
kevinrenskers added a commit that referenced this pull request Aug 24, 2022
@kevinrenskers
feat: Remove timezone from device context
1d48c94 
Continuation of #2036

#skip-changelog
@kevinrenskers kevinrenskers mentioned this pull request Aug 24, 2022
Merged
@armcknight armcknight mentioned this pull request Dec 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@armcknight armcknight armcknight approved these changes

@philipphofmann philipphofmann Awaiting requested review from philipphofmann

@brustolin brustolin Awaiting requested review from brustolin

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

Successfully merging this pull request may close these issues.

Add Event Culture Context
3 participants
@kevinrenskers @armcknight @getsentry-bot