Add the Go Modules builder #65
Conversation
volkangurel
force-pushed
the
feature/go-modules-builder
branch
3 times, most recently
from
a543684
to
62c5048
Compare
volkangurel
force-pushed
the
feature/go-modules-builder
branch
from
c1e6a20
to
e594bac
Compare
Add a builder for Go that assumes the given go project uses Go Modules, and so can be built using standard go tooling.
Update the go models path resolvers and validators to the latest framework merged in aws#55
volkangurel
force-pushed
the
feature/go-modules-builder
branch
from
84f2e68
to
dd837e0
Compare
|
|
||
| mismatched = p.returncode != 0 \ | ||
| or len(out.split()) < 3 \ | ||
| or out.split()[2].replace(self.language, "").split('.')[0] != expected_major_version |
There was a problem hiding this comment.
Will the output of go version always be in the form of go version go1.10.3 darwin/amd64?
Go Modules is only available with go 1.11 or greater, should we have that as part of this validation?
There was a problem hiding this comment.
Yes, great point, I'll add that to the validator.
|
|
||
| mismatched = p.returncode != 0 \ | ||
| or len(out.split()) < 3 \ | ||
| or out.split()[2].decode().replace(self.language, "").split('.')[0] != expected_major_version |
There was a problem hiding this comment.
Go Module is only supported for go versions greater than 1.11 right? Shouldn't this be checking for that instead of just the major version?
| out, _ = p.communicate() | ||
|
|
||
| mismatched = p.returncode != 0 \ | ||
| or len(out.split()) < 3 \ |
There was a problem hiding this comment.
On my mac:
$ go version
go version go1.10.3 darwin/amd64
That will fail this check. What is the importance of this check?
There was a problem hiding this comment.
Where out = "go version go1.10.3 darwin/amd64", the len(out.split()) < 3 expression will return False, but that's expected since this is a chained or expression where any truthy part would set mismatched to True and raise an exception later. I can definitely make this clearer to avoid confusion.
There was a problem hiding this comment.
Ahh this was a guard for getting the version out of the go version command. I didn't realize this initially.
requirements/base.txt
Outdated
| @@ -1 +1,2 @@ | |||
| six~=1.11 | |||
| whichcraft~=0.5.2 | |||
There was a problem hiding this comment.
This isn't needed anymore right?
|
@volkangurel Are you submitting this on behalf of Coinbase? If so, can you confirm you have approval from the company? |
|
@jfuss I'm happy to submit it either on behalf of Coinbase or myself. Our company policy is: |
|
|
||
| @property | ||
| def validated_runtime_path(self): | ||
| return self._valid_runtime_path if self._valid_runtime_path is not None else None |
There was a problem hiding this comment.
self._valid_runtime_path defaults to None. This can be simplified to just return self._valid_runtime_path.
There was a problem hiding this comment.
Good call! I'll fix this. I can also fix where I copied this from: https://github.com/awslabs/aws-lambda-builders/blob/f53dc86b9094bdae5fdc36d702b4d8c093c06ed7/aws_lambda_builders/workflows/python_pip/validator.py#L73.
|
@volkangurel Please confirm this contribution is under the terms of the Apache 2.0 license. Thanks. |
|
@jfuss I confirm that my contribution is made under the terms of the Apache 2.0 license. |
Issue #, if available:
#64
Description of changes:
Add a builder for Go that assumes the given go project uses Go Modules, and so
can be built using standard go tooling.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.