Trent/image ci #25
Trent/image ci #25
Conversation
![codecov[bot]](https://avatars.githubusercontent.com/in/254?s=60&v=4){kind=small}
.github/workflows/test-api-ci.yml
Outdated
| docker_path: integration-tests/test-api/Dockerfile | ||
| push: | ||
| name: Build API Test | ||
| if: ${{ github.event_name == 'push' && github.event.ref == 'refs/heads/main' && github.repository_owner == 'codecov' }} |
There was a problem hiding this comment.
This expression seems too restrictive. For example, it strictly checks for the repository owner to be 'codecov'. If the repository is forked, the owner would change and this would stop working. Removing ' && github.repository_owner == 'codecov'' might improve flexibility.
There was a problem hiding this comment.
@eliatcodecov Medium helpful comment. The bot is probably right here.
Makefile
Outdated
| export DOCKER_BUILDKIT := 1 | ||
|
|
||
| build.${IMAGE_NAME}: | ||
| docker build -f ${DOCKER_PATH} . -t ${dockerhub_image}:latest \ |
There was a problem hiding this comment.
The variable 'sha', 'build_date', 'release_version' are not defined in Makefile but are used. These need to be defined or managed effectively.
There was a problem hiding this comment.
@eliatcodecov actually helpful comment!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
☔ View full report in Codecov by Sentry. |
Makefile
Outdated
| OS_NAME := $(shell uname -s | tr A-Z a-z) | ||
| BUILD_PLATFORM := | ||
| ifeq ($(OS_NAME),darwin) | ||
| BUILD_PLATFORM := arm64 |
There was a problem hiding this comment.
The BUILD_PLATFORM is being set to 'arm64' for Darwin systems. However, not all Darwin systems are arm64, some are x86_64 (Intel). Consider a more comprehensive approach to accurately determine the architecture.
There was a problem hiding this comment.
@eliatcodecov this is actually a really good one
| push: | ||
| name: Push API Test | ||
| if: github.repository_owner == 'codecov' | ||
| uses: codecov/gha-workflows/.github/workflows/[email protected] |
There was a problem hiding this comment.
Like in the above file, the 'secrets' keyword does not seem to be used correctly. Secrets can be accessed using the secrets context but need to be passed to the env attribute.
There was a problem hiding this comment.
@eliatcodecov The bot is getting snarky
Description
Add CI for test-api image
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.