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

Allows merging user context on Scope::setUser() #931

Merged
merged 18 commits into from
Dec 16, 2019
Merged

Allows merging user context on Scope::setUser() #931

merged 18 commits into from
Dec 16, 2019

Conversation

volkyeth
Copy link
Contributor

Closes #929

Copy link
Contributor

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Good PR! LGTM 👍

@ste93cry
Copy link
Contributor

I just noticed that this PR is pointing to the wrong branch. Since it’s not a bug fix it should target develop. @brunowowk would you mind to switch target branch and in case rebase?

@volkyeth volkyeth changed the base branch from master to develop December 11, 2019 17:43
Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

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

Small documentation nitpick but looks solid! 🚢

@volkyeth volkyeth changed the title Allows merging user context on Scope::setUser(). Adds Scope::clearUser() Allows merging user context on Scope::setUser(). Adds Scope::removeUser() Dec 12, 2019
@volkyeth volkyeth changed the title Allows merging user context on Scope::setUser(). Adds Scope::removeUser() Allows merging user context on Scope::setUser() Dec 16, 2019
Copy link
Contributor

@ste93cry ste93cry left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution and the patience, very good work! LGTM

@ste93cry ste93cry merged commit 910a255 into getsentry:develop Dec 16, 2019
@volkyeth volkyeth deleted the merge-user-context branch December 16, 2019 13:24
@pilif
Copy link
Contributor

pilif commented Jan 16, 2020

Add the Scope::clearUser method to clear user context. (#929)

this seems to not have been part of the PR - what is the recommended way now to set a user to specific value without merging?

@ste93cry
Copy link
Contributor

If you mean how you can remove an item from the root of the user context, there is no way to do it now. If you need to push some data and clear it after some operation finished the recommended way is to use a new scope with \Sentry\withScope

@abramov-ks
Copy link

So, how I need to set up user data when I try to initialize Sentry manualy in my project ?

@ste93cry
Copy link
Contributor

It depends on what you are trying to do. Please take a look at the documentation, and if something is not clear feel free to open a new issue and/or write to the support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants