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

Cache rebased #312

Merged
merged 4 commits into from Apr 3, 2017

Conversation

Projects
None yet
5 participants
@harikt
Contributor

harikt commented Mar 23, 2017

For #309

harikt added some commits Mar 21, 2017

Don't serialize, just test if cache exists
Add unit tests to cache build and clear.
{
$this
->setName('orm-cache-build')
->setDescription('Build all metadata caches for the connection. If a table name is provided, only that table will be cached.')

This comment has been minimized.

@stickler-ci

stickler-ci Mar 23, 2017

Line exceeds 120 characters; contains 138 characters

@stickler-ci

stickler-ci Mar 23, 2017

Line exceeds 120 characters; contains 138 characters

This comment has been minimized.

@harikt

harikt Mar 23, 2017

Contributor

I wished if the @stickler-ci could fix it than complaining.

@harikt

harikt Mar 23, 2017

Contributor

I wished if the @stickler-ci could fix it than complaining.

This comment has been minimized.

@lorenzo

lorenzo Apr 3, 2017

Member

@markstory not a bad idea ^

@lorenzo

lorenzo Apr 3, 2017

Member

@markstory not a bad idea ^

{
$this
->setName('orm-cache-clear')
->setDescription('Clear all metadata caches for the connection. If a table name is provided, only that table will be removed.')

This comment has been minimized.

@stickler-ci

stickler-ci Mar 23, 2017

Line exceeds 120 characters; contains 139 characters

@stickler-ci

stickler-ci Mar 23, 2017

Line exceeds 120 characters; contains 139 characters

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Mar 23, 2017

Codecov Report

Merging #312 into master will increase coverage by 0.1%.
The diff coverage is 88.73%.

@@            Coverage Diff            @@
##           master     #312     +/-   ##
=========================================
+ Coverage   85.97%   86.07%   +0.1%     
=========================================
  Files          33       36      +3     
  Lines        1782     1853     +71     
=========================================
+ Hits         1532     1595     +63     
- Misses        250      258      +8
Impacted Files Coverage Δ
src/MigrationsDispatcher.php 100% <100%> (ø) ⬆️
src/Util/SchemaTrait.php 57.14% <57.14%> (ø)
src/Command/CacheBuild.php 96.15% <96.15%> (ø)
src/Command/CacheClear.php 96.55% <96.55%> (ø)

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 978bef8...bd06673. Read the comment docs.

codecov-io commented Mar 23, 2017

Codecov Report

Merging #312 into master will increase coverage by 0.1%.
The diff coverage is 88.73%.

@@            Coverage Diff            @@
##           master     #312     +/-   ##
=========================================
+ Coverage   85.97%   86.07%   +0.1%     
=========================================
  Files          33       36      +3     
  Lines        1782     1853     +71     
=========================================
+ Hits         1532     1595     +63     
- Misses        250      258      +8
Impacted Files Coverage Δ
src/MigrationsDispatcher.php 100% <100%> (ø) ⬆️
src/Util/SchemaTrait.php 57.14% <57.14%> (ø)
src/Command/CacheBuild.php 96.15% <96.15%> (ø)
src/Command/CacheClear.php 96.55% <96.55%> (ø)

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 978bef8...bd06673. Read the comment docs.

@harikt

This comment has been minimized.

Show comment
Hide comment
@harikt

harikt Mar 29, 2017

Contributor

Is there anything I can do to make this merge ?

Contributor

harikt commented Mar 29, 2017

Is there anything I can do to make this merge ?

@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Apr 3, 2017

Member

I'll merge now but not doing a release until I confirm it does not create a big conflict with the shells provided by the cake core

Member

lorenzo commented Apr 3, 2017

I'll merge now but not doing a release until I confirm it does not create a big conflict with the shells provided by the cake core

@lorenzo lorenzo merged commit b708eed into cakephp:master Apr 3, 2017

3 of 4 checks passed

stickler-ci Lint errors found, see pull request comments.
codecov/patch 88.73% of diff hit (target 85.97%)
Details
codecov/project 86.07% (+0.1%) compared to 978bef8
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Apr 3, 2017

Member

@harikt thanks so much!!

Member

lorenzo commented Apr 3, 2017

@harikt thanks so much!!

@harikt harikt deleted the harikt:cache-rebased branch Apr 3, 2017

@harikt

This comment has been minimized.

Show comment
Hide comment
@harikt

harikt Apr 3, 2017

Contributor

@lorenzo thank you for merging this.

I confirm it does not create a big conflict with the shells provided by the cake core

I don't believe so. But you can test things before the release :-) .

thanks so much!!

You are very welcome. And thank you once again for the merge. I hate to see PR's waiting for days :-) .

Contributor

harikt commented Apr 3, 2017

@lorenzo thank you for merging this.

I confirm it does not create a big conflict with the shells provided by the cake core

I don't believe so. But you can test things before the release :-) .

thanks so much!!

You are very welcome. And thank you once again for the merge. I hate to see PR's waiting for days :-) .

@HavokInspiration HavokInspiration modified the milestone: 1.6.8 Jun 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment