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

[FIX] adapters/Memory: Return cloned resources #235

Merged
merged 4 commits into from
Jun 4, 2020
Merged

Conversation

matz3
Copy link
Member

@matz3 matz3 commented Apr 22, 2020

Thank you for your contribution! 🙌

To get it merged faster, kindly review the checklist below:

Pull Request Checklist

@coveralls
Copy link

coveralls commented Apr 29, 2020

Coverage Status

Coverage increased (+0.2%) to 87.35% when pulling 1dbd2a9 on fix-memory-adapter into a0205f6 on master.

@matz3 matz3 force-pushed the fix-memory-adapter branch from f1e522f to 09e6ba2 Compare April 29, 2020 15:47
@RandomByte
Copy link
Member

Missing tests

@matz3
Copy link
Member Author

matz3 commented May 5, 2020

True. I've just made sure that existing tests do not break, but we should also have new tests. I'll take care of it.

@matz3 matz3 force-pushed the fix-memory-adapter branch from 09e6ba2 to 1dbd2a9 Compare June 4, 2020 13:57
@matz3
Copy link
Member Author

matz3 commented Jun 4, 2020

Rebased + added tests

@matz3 matz3 added the bug Something isn't working label Jun 4, 2020
@matz3 matz3 merged commit 7bf3c6a into master Jun 4, 2020
@matz3 matz3 deleted the fix-memory-adapter branch June 4, 2020 19:52
@RandomByte
Copy link
Member

As discussed today, we should also clone on write in order to have a similar behavior as with the FS Adapter where for example a resources content can't be changed after it has been written.

On read however, we can use the createStream parameter to skip copying a resources content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants