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

wgetopt variable for general used wget options #214

Merged
merged 1 commit into from
May 2, 2017
Merged

Conversation

waja
Copy link
Contributor

@waja waja commented Mar 19, 2017

Makes it easier to change (and replace) wget options at a unique place.

@rfxn
Copy link
Owner

rfxn commented Mar 20, 2017

Thanks for the contribution.

I want to say there was a reason I didn't do this initially but I can not recall it offhand, so it couldn't have been very compelling.... I will review this when time permits in coming days and commit for a quick 1.6.1 release I need to put out to address some smaller items.

@waja
Copy link
Contributor Author

waja commented Mar 20, 2017

I stumbled upon this when looking into the possibilities to replace wget with curl. But this task needs a bit more work I think.

@@ -27,6 +27,7 @@ wget_retries="3"
wget_proxy=""

wget=`which wget 2> /dev/null`
wgetopt=`-e http_proxy="$wget_proxy" -e https_proxy="$wget_proxy" -T$wget_timeout -t$wget_retries -q`
Copy link
Owner

@rfxn rfxn May 1, 2017

Choose a reason for hiding this comment

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

wgetopt= is defined with backticks = will try run as a command instead of defining as a variable

Can you fix this up? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry ... that slipped somehow through. I've converted them into single quotes.

Makes it easier to change (and replace) wget options at a unique place.
@waja
Copy link
Contributor Author

waja commented May 2, 2017

I pushed an updated commit. I'm not able to assign the PR back to you @rfxn

@rfxn rfxn merged commit 8446a66 into rfxn:master May 2, 2017
@waja waja deleted the wget_unify branch May 2, 2017 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants