Skip to content

Add support for HTTPlug 2.0 #24

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

Closed
wants to merge 1 commit into from
Closed

Conversation

GeorgosXL
Copy link

No description provided.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@codecov-io
Copy link

Codecov Report

Merging #24 into 2.x will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##                2.x      #24   +/-   ##
=========================================
  Coverage     86.35%   86.35%           
  Complexity      627      627           
=========================================
  Files            92       92           
  Lines          1370     1370           
=========================================
  Hits           1183     1183           
  Misses          187      187

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 564ea5a...01dc3a1. Read the comment docs.

@mxr576
Copy link
Contributor

mxr576 commented Oct 31, 2018

Wow, HTTPlug 2.0 has just released. That was quick... :)

Thanks @GeorgosXL for your PR. Please sign the CLA and submit a link to a Travis build that proof your changes does not break any tests even when the online client is being used. (PR tests use mock responses, they do not call the Apigee Edge API.)
These are the variables that you have to configure to run a build in Travis:
https://github.com/apigee/apigee-client-php#unit-tests

Expected link is something like this: https://travis-ci.org/mxr576/apigee-client-php/builds/448781277

@mxr576 mxr576 changed the title support for httplug 2.0 Add support for HttpPlug 2.0 Oct 31, 2018
@mxr576 mxr576 changed the title Add support for HttpPlug 2.0 Add support for HTTPlug 2.0 Oct 31, 2018
@mxr576 mxr576 requested review from mxr576 and cnovak October 31, 2018 11:06
@mxr576
Copy link
Contributor

mxr576 commented Oct 31, 2018

Related: php-http/client-common#114

So we should not hurry with this merge of this PR because there are plenty php-http components that version needs to be bumped and for that they need to be compatible with HttpPlug 2.0 (precisely the finalized PSR-18).

@mxr576 mxr576 added the beta blocker For issues/PRs that should be address before beta1. label Oct 31, 2018
@Jaesin
Copy link
Contributor

Jaesin commented Nov 9, 2018

Besides the CLA, I think it would be fair to just do a manual review for a small PR like this one. Of course the dependency that changed should be reviewed as well.

@mxr576
Copy link
Contributor

mxr576 commented Nov 12, 2018

@Jaesin You may think a simple version bump is a "small" change, but we have even seen recently that a simple minor version (reminder, this is a major version update) about in PHP API client's dependencies can break the library. See #20. I though it is going to be a simple update but luckily our online test suite that Travis executes revealed that if we would update to the latest major version from the php-client/client-common the PHP API client would stop working.
So we should not make exceptions because setting up the test suite is fairly easy in Travis. If you do not think it is easy enough please provide suggestions about how we should improve our CONTRIBUTING.md. :)

@Jaesin
Copy link
Contributor

Jaesin commented Nov 14, 2018

mxr576 added a commit to mxr576/apigee-edge-drupal that referenced this pull request Nov 19, 2018
could not be installed with apigee/apigee-client-php 2.0.0-alpha6
version.

The upgrade to the 2.0 version should eliminate this problem:
apigee/apigee-client-php#24
@mxr576
Copy link
Contributor

mxr576 commented Nov 20, 2018

Client common library's 2.0 version is still in progress: php-http/client-common#115

@mxr576 mxr576 removed the beta blocker For issues/PRs that should be address before beta1. label Jan 18, 2019
@mxr576
Copy link
Contributor

mxr576 commented Jan 22, 2019

It seems we can not support 2.x version from php-http packages without releasing a new major version from this library. I am closing this PR and opening an issue where I try to collect all info that I have regarding this.

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

Successfully merging this pull request may close these issues.

5 participants