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

Implement strategy A #32

Closed
wants to merge 1 commit into from
Closed

Implement strategy A #32

wants to merge 1 commit into from

Conversation

mnapoli
Copy link
Member

@mnapoli mnapoli commented Apr 18, 2020

Following the discussion in #21, I started working yesterday on this. (this is why I opened the PRs for the new tests)

I have opened #31 to sort things out, I pushed my branch to illustrate.

Locally the tests are passing except the tests that try to write to the cache pools. That is expected, I haven't added any change in configuration for the cache configuration (this is what I intend to test down the road if we decide on that strategy).

@mnapoli mnapoli mentioned this pull request Apr 18, 2020
The compiled container is used from the deployed cache directory (if it exists).
@Nyholm
Copy link
Collaborator

Nyholm commented Apr 18, 2020

Im 👎 for this. It removes all possibility to control what's writable or not.

If a user would like this, they wouldn't use this library, they would just do the 3 line change themselves.

I also dont see how this is better or more performant than the current code in master.

@mnapoli mnapoli closed this Dec 30, 2021
@mnapoli mnapoli deleted the no-copying branch December 30, 2021 15:09
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.

2 participants