Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ref: Make getScope private #776

Merged
merged 5 commits into from
Feb 22, 2019
Merged

ref: Make getScope private #776

merged 5 commits into from
Feb 22, 2019

Conversation

HazAT
Copy link
Member

@HazAT HazAT commented Feb 22, 2019

No description provided.

@HazAT HazAT self-assigned this Feb 22, 2019
Copy link
Collaborator

@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.

LGTM, apart a small comment, and the need to add this to the changelog.

tests/State/HubTest.php Outdated Show resolved Hide resolved
@HazAT HazAT requested a review from Jean85 February 22, 2019 18:44
Copy link
Collaborator

@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.

LGTM 👍

I've added the changelog myself, also with other (already merged) missing stuff.

tests/State/HubTest.php Outdated Show resolved Hide resolved
tests/State/HubTest.php Outdated Show resolved Hide resolved
tests/SdkTest.php Outdated Show resolved Hide resolved
tests/ClientTest.php Outdated Show resolved Hide resolved
@HazAT HazAT requested a review from ste93cry February 22, 2019 21:30
@ste93cry
Copy link
Collaborator

I don't really like that we have to use reflection to test the internal behavior of an object, however at the moment I don't see any other way around so LGTM

@ste93cry ste93cry merged commit aac8588 into master Feb 22, 2019
@ste93cry ste93cry deleted the ref/private-get-scope branch February 22, 2019 22:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants