Skip to content

Comments

Move required dependencies to Composer require#1342

Merged
kstich merged 4 commits intomasterfrom
updateComposerDependencies
Aug 22, 2017
Merged

Move required dependencies to Composer require#1342
kstich merged 4 commits intomasterfrom
updateComposerDependencies

Conversation

@imshashank
Copy link
Contributor

@imshashank imshashank commented Aug 4, 2017

Fixes #1285
Fixes #1341

@codecov-io
Copy link

codecov-io commented Aug 4, 2017

Codecov Report

Merging #1342 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1342      +/-   ##
============================================
- Coverage     91.87%   91.86%   -0.02%     
- Complexity     2466     2471       +5     
============================================
  Files           142      142              
  Lines          7523     7544      +21     
============================================
+ Hits           6912     6930      +18     
- Misses          611      614       +3
Impacted Files Coverage Δ Complexity Δ
src/Sdk.php 87.27% <0%> (-0.97%) 19% <0%> (+5%)

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 e49a541...0679fb5. Read the comment docs.

Copy link
Contributor

@kstich kstich left a comment

Choose a reason for hiding this comment

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

This needs a Changelog Document. Because this is an update to the core of the SDK in terms of requirements, it should be messaged as a "feature" sized update to the core in the changelog document.

This was a misclassification going all the way back to v3.0.0 - it's even inconsistent with the associated compatibility-test file in terms of the required extensions. Since we don't have hard requirements on curl and dom, these should be moved to suggested installs in the compatibility test file as well.

@imshashank
Copy link
Contributor Author

@kstich Updated the test and compatibility code, please let me know if anything else required.

@imshashank imshashank changed the title Move required dependencies to composer require Move required dependencies to Composer require Aug 21, 2017
@kstich kstich merged commit 99fd287 into master Aug 22, 2017
@kstich kstich deleted the updateComposerDependencies branch September 12, 2017 19:43
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.

3 participants