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

remove sed from scripts #38

Open
felixgao opened this issue Jul 7, 2016 · 4 comments
Open

remove sed from scripts #38

felixgao opened this issue Jul 7, 2016 · 4 comments
Labels

Comments

@felixgao
Copy link
Contributor

felixgao commented Jul 7, 2016

most web projects do not have sed in their scripts. let's try to remove it.

@jwtodd
Copy link
Member

jwtodd commented Jul 8, 2016

i don't understand the premise, in that a simple google search turns up dockerfile+sed a plenty:

https://www.google.com/search?q=dockerfile+sed&oq=dockerfile+sed&aqs=chrome..69i57.2036j0j1&sourceid=chrome&ie=UTF-8

what is the real concern?

i personally do not feel this is a significant concern.

very open to improvements, i just would like to understand the concrete concerns/objectives.

thoughts?

@felixgao
Copy link
Contributor Author

Just because people are doing it does not mean it is the right thing to do. The concern is docker uses something called aufs which is a write once file system. Have sed in the script will cause docker to generate a new layer and the more layer there is in the docker image the slower it is and the more file system resource it takes. Also having sed means you are not make the image completely immutable which is a bad practice as well.

@iizrailevsky
Copy link
Member

@felixgao Mind creating a pull request with proposed changes to the script(s)? As we had discussed, we own these together as Wasabi committers. If you see areas of improvement, you're empowered to work on the updates to the artifacts and propose them as a PR.

@shoeffner
Copy link
Member

@felixgao do you mind taking a look at https://github.com/intuit/wasabi/pull/78/files#diff-a02022c11f2de6a41f82f8a76f5e0d3cR19 ? It still contains sed in the Dockerfile but much fewer layers around it. Otherwise how should we approach this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants