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

add .run_line_wrap helper function to wrap long CMD lines in dockerfiles #128

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

e-kotov
Copy link
Contributor

@e-kotov e-kotov commented Apr 13, 2023

Currently the dockerize() function produces rather unfriendly long lines of RUN commands. For example:

RUN apt-get update -qq && apt-get install -y libpcre3-dev zlib1g-dev pkg-config libcurl4-openssl-dev && apt-get install -y libcurl4-openssl-dev libicu-dev libssl-dev make zlib1g-dev

With the addition of the new internal (non-exported) helper function .run_line_wrap() this becomes a more conventional and human readable:

RUN apt-get update -qq \
	&& apt-get install -y libpcre3-dev zlib1g-dev pkg-config libcurl4-openssl-dev \
	&& apt-get install -y libcurl4-openssl-dev libicu-dev libssl-dev make zlib1g-dev

@codecov-commenter
Copy link

Codecov Report

Merging #128 (45fe555) into v0.3 (0866aad) will increase coverage by 0.00%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             v0.3     #128   +/-   ##
=======================================
  Coverage   97.26%   97.27%           
=======================================
  Files          10       10           
  Lines         989      991    +2     
=======================================
+ Hits          962      964    +2     
  Misses         27       27           
Impacted Files Coverage Δ
R/dockerfile.R 100.00% <100.00%> (ø)
R/installation.R 97.43% <100.00%> (+0.02%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@chainsawriot
Copy link
Collaborator

@e-kotov Thanks a lot for the PR! I have provide the CR. Please let me know if you need help with the requested changes.

@e-kotov
Copy link
Contributor Author

e-kotov commented Apr 13, 2023

@chainsawriot I incorporated your review suggestions and written a test. I could not come up with the one that would use writeLines(). To me, it makes sense to check if all lines with && start with \t && and that there are no lines where there are two or more occurrences of &&.

@chainsawriot chainsawriot merged commit bedfb4d into gesistsa:v0.3 Apr 13, 2023
@chainsawriot
Copy link
Collaborator

@e-kotov Thanks a lot and I will add you as a CTB.

chainsawriot added a commit that referenced this pull request Apr 13, 2023
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.

3 participants