Skip to content

make things private when possible #89

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
Merged
merged 1 commit into from
Jun 16, 2014
Merged

make things private when possible #89

merged 1 commit into from
Jun 16, 2014

Conversation

@dbu
Copy link
Contributor

@dbu dbu commented Jun 16, 2014

dbu
dbu reviewed Jun 16, 2014
View reviewed changes
src/ProxyClient/AbstractProxyClient.php
Copy link
Contributor Author

@dbu dbu Jun 16, 2014

Choose a reason for hiding this comment

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

is this ok?

Copy link
Member

@ddeboer ddeboer Jun 16, 2014

Choose a reason for hiding this comment

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

Yes. If we go private, let's go private all the way. :)

@dbu
Copy link
Contributor Author

dbu commented Jun 16, 2014

AbstractProxyClient also has protected function queueRequest($method, $url, array $headers = array()) which is a bit awkward as it can't be used to queue an arbitrary request. but it needs to be protected, as is createRequest as that needs to be customized by the concrete implementations

@dbu
Copy link
Contributor Author

dbu commented Jun 16, 2014

btw, tests on FOSHttpCacheBundle success with this branch. Lets see for the lib itself with travis.

@ddeboer
Copy link
Member

ddeboer commented Jun 16, 2014

I guess the protected queueRequest() is okay for now. Let's merge this.

dbu added a commit that referenced this pull request Jun 16, 2014
@dbu
Merge pull request #89 from FriendsOfSymfony/protected-private
d901fa8 
make things private when possible
@dbu dbu merged commit d901fa8 into master Jun 16, 2014
@dbu dbu deleted the protected-private branch June 16, 2014 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dbu @ddeboer