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

Code optimization #114

Merged
merged 4 commits into from
Dec 10, 2018
Merged

Code optimization #114

merged 4 commits into from
Dec 10, 2018

Conversation

mrcnpdlk
Copy link
Contributor

@mrcnpdlk mrcnpdlk commented Nov 28, 2018

Code optimization using PHPStorm plugin.

  • improve performace
  • minor fixes

@codecov
Copy link

codecov bot commented Nov 28, 2018

Codecov Report

Merging #114 into master will not change coverage.
The diff coverage is 91.17%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #114   +/-   ##
=========================================
  Coverage     88.21%   88.21%           
+ Complexity      244      242    -2     
=========================================
  Files            15       15           
  Lines           789      789           
=========================================
  Hits            696      696           
  Misses           93       93
Impacted Files Coverage Δ Complexity Δ
lib/MessageDecoratorTrait.php 100% <ø> (ø) 15 <0> (ø) ⬇️
lib/Auth/AWS.php 86.48% <100%> (ø) 23 <0> (ø) ⬇️
lib/Request.php 100% <100%> (ø) 23 <0> (-1) ⬇️
lib/Response.php 100% <100%> (ø) 14 <0> (-1) ⬇️
lib/Message.php 100% <100%> (ø) 30 <0> (ø) ⬇️
lib/Sapi.php 95.74% <100%> (ø) 34 <0> (ø) ⬇️
lib/functions.php 95.48% <100%> (ø) 0 <0> (ø) ⬇️
lib/Client.php 56.96% <66.66%> (ø) 49 <0> (ø) ⬇️
lib/Auth/Digest.php 96.29% <80%> (ø) 19 <0> (ø) ⬇️

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 3577f01...f070a11. Read the comment docs.

@staabm
Copy link
Member

staabm commented Nov 29, 2018

IMO we should drop the 2 commits we discussed. otherwise this looks good to me.

thx for the PR.

@mrcnpdlk
Copy link
Contributor Author

I've removed these two commits. Now everything should be OK.
Please add cookbook for contributors.

@DeepDiver1975
Copy link
Member

can I ask you to squash the commits a bit into reasonable units? THX

Direct comparison is faster

Type casting is faster
unnecessary type casting

redundant else keyword

resolving tests errors - type casting

pretty fix

pretty fix
@staabm staabm merged commit 0e55cc6 into sabre-io:master Dec 10, 2018
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.

4 participants