-
Notifications
You must be signed in to change notification settings - Fork 6
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
Added serverless #26
Added serverless #26
Conversation
/test |
@shawju make sure you have your settings correct: #23 (comment) |
/test |
…and added serverless
…and added serverless
/test |
/test |
1 similar comment
/test |
/test |
/test |
/test |
/test |
/test |
/test |
/test |
1 similar comment
/test |
/test |
2 similar comments
/test |
/test |
/test |
/test |
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Delete src/.DS_Store
Other than that it LGTM
I'm reviewing from a compliance perspective only. Should get another approval from anvil team before merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be adding hadolint
ignores unless we explicitly need the acception.
src/docker/Dockerfile
Outdated
@@ -85,7 +86,7 @@ RUN wget -O /root/task.tar.gz "https://github.com/go-task/task/releases/download | |||
# Install shellcheck. Get versions from https://github.com/koalaman/shellcheck/releases | |||
ARG SHELLCHECK_VERSION="0.7.1" | |||
ENV SHELLCHECK_VERSION=${SHELLCHECK_VERSION} | |||
# hadolint ignore=DL3003 | |||
# hadolint ignore=DL3003,DL3047 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want this ignore here. We should update wget
to use the --progress
flag.
See here
src/docker/Dockerfile
Outdated
@@ -183,20 +195,23 @@ RUN asdf plugin add python \ | |||
# NOTE: PIPENV_VERSION is an env var that is used by pipenv, so it can't be used here. ref: https://github.com/pypa/pipenv/issues/3633#issuecomment-478250721 | |||
ARG PIP_ENV_VERSION="2020.11.15" | |||
ENV PIP_ENV_VERSION=${PIP_ENV_VERSION} | |||
# hadolint ignore=DL3042 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want this ignore here.
See here
RUN dnf -y install skopeo-${SKOPEO_VERSION} \ | ||
&& dnf clean all \ | ||
&& rm -rf /var/cache/yum/ \ | ||
&& rm -rf /var/tmp/* /tmp/* /var/tmp/.???* /tmp/.???* | ||
|
||
# hadolint ignore=DL3059 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do?
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
spec/skopeo_spec.sh
Outdated
The output should include "${SKOPEO_VERSION}" | ||
The status should eq 0 | ||
End | ||
It "validates tool is installed by checking version" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing
This file needs to be indented like the other shellspec
files
/test |
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving from compliance perspective only. Get a 2nd from @saic-oss/anvil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
what
why
references