search

lifadev / archive_aws-lambda-go-shim

Author your AWS Lambda functions in Go, effectively.
https://github.com/eawsy/aws-lambda-go-shim
Apache License 2.0
789 stars 66 forks source link

Quote LDFLAGS in Makefile.example #35

Closed cristim closed 7 years ago

cristim commented 7 years ago

I think it needs to be quoted in order to properly support the case when LDFLAGS contains multiple options separated by whitespace.

fsenart commented 7 years ago

It already works if the user gives the var as follows: LDFLAGS='"foo bar"' make. You think that it's not up to the user to do this? Even in Go, when giving the -ldflags one has to quote the value if there are multiple values. Go does not do the job for the end user.

cristim commented 7 years ago

I find it cleaner to quote/fix it where it's needed as quoted (the docker run command fails without quotes) instead of defining it with nested quotes from the start.

Without the quotes in the Makefile.example, my main Makefile would need to define it like this, which I personally find ugly:

LDFLAGS="'-pluginpath lambda -X lambda.Version=$(BUILD)'"

Later edit: never mind, I found a way to make my Makefile quote it.

fsenart commented 7 years ago

Hey @cristim why did you closed the PR, I was performing some tests in order to know if this modification is not too specific for those (like you) who use another Makefile to run the Makefile.example. It had to be generic for every usage. So I just finished my tests and it's very ok. So please reopen so I can merge :wink:

fsenart commented 7 years ago

Oh you've deleted it in a row :cry: Reopen it please.

cristim commented 7 years ago

@fsenart reopened