Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

[FIX] wkhtmltox build arg #451

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lasley
Copy link
Contributor

@lasley lasley commented Jun 16, 2017

  • Add missing WKHTMLTOPDF_VERSION declaration and use a SKIP arg instead of blank

This env var was introduced in #449, but doesn't exist anywhere.

Additionally - In order to avoid having to change all our Travis files, I propose that we skip the installation with the value SKIP, otherwise we install the version we have defined (0.12.4)

@lasley lasley requested a review from moylop260 June 16, 2017 20:16
@@ -82,7 +84,7 @@ if [ "$clone_result" != "0" ]; then
exit $clone_result
fi;

if [ "${WKHTMLTOPDF_VERSION}" != "" ]; then
if [ "${WKHTMLTOPDF_VERSION}" != "SKIP" ]; then
echo "Install webkit (wkhtmltopdf) patched version ${WKHTMLTOPDF_VERSION}"
(cd ${HOME}/maintainer-quality-tools/travis/ && wget -qO- -t 1 --timeout=240 https://downloads.wkhtmltopdf.org/0.12/${WKHTMLTOPDF_VERSION}/wkhtmltox-${WKHTMLTOPDF_VERSION}_linux-generic-amd64.tar.xz | tar -xJ --strip-components=2 wkhtmltox/bin/wkhtmltopdf)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the url to use github here? (without checksum process)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you said to leave that in #450? That's the only thing that PR consists of now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diffs should align I think regardless of merge order due to not editing the same or bordering lines?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The travis is red IMHO we need change the url to use the new github URL (if we are forcing a installation by default)

@lasley lasley force-pushed the bugfix/run-wkhtmltox-install branch 2 times, most recently from 1d19277 to fd21163 Compare June 18, 2017 23:28
@lasley
Copy link
Contributor Author

lasley commented Jun 18, 2017

Alright this should be 🍏 now

* Add missing WKHTMLTOPDF_VERSION declaration and use a SKIP arg instead of blank
@lasley lasley force-pushed the bugfix/run-wkhtmltox-install branch from fd21163 to 7f6bd3b Compare January 3, 2018 00:36
@lasley
Copy link
Contributor Author

lasley commented Jan 3, 2018

Rebased to solve conflicts.

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

Successfully merging this pull request may close these issues.

2 participants