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

Exposes docker env variables to gradle steps #47

Merged
merged 1 commit into from
Dec 22, 2020
Merged

Exposes docker env variables to gradle steps #47

merged 1 commit into from
Dec 22, 2020

Conversation

JBAhire
Copy link
Member

@JBAhire JBAhire commented Dec 22, 2020

Description

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

Documentation

https://github.com/hypertrace/hypertrace-gradle-docker-plugins/blob/c4cbaa615a03724c17c9eed0decf7311ac8128dc/hypertrace-gradle-docker-plugin/README.md#other-configuration

@JBAhire JBAhire changed the title Exposes env variables to gradle steps Exposes docker env variables to gradle steps Dec 22, 2020
run: ./gradlew dockerPushImages
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still docker login after adding env here? If, not, we can remove that in follow-up PR.

@JBAhire JBAhire merged commit cb6935d into main Dec 22, 2020
@JBAhire JBAhire deleted the cleanup-gha branch December 22, 2020 07:50
@@ -67,6 +60,6 @@ jobs:
- name: Archive integration test report
uses: actions/upload-artifact@v1
with:
name: integration-test
path: /tmp/integration-test-reports
name: test-reports
Copy link
Contributor

@kotharironak kotharironak Dec 22, 2020

Choose a reason for hiding this comment

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

Curious (not a blocker): Why are we uploading as artifacts when we are uploading to codecov?

Copy link
Contributor

Choose a reason for hiding this comment

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

codecov gives us test coverage but not test results

@@ -33,7 +33,7 @@ jobs:
run: ./gradlew jacocoTestReport

- name: Copy unit test reports
run: ./gradlew copyAllReports --output-dir=/tmp/unit-test-reports
run: ./gradlew copyAllReports --output-dir=/tmp/test-reports
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be omitted, it's being run later

@@ -67,6 +60,6 @@ jobs:
- name: Archive integration test report
uses: actions/upload-artifact@v1
with:
name: integration-test
path: /tmp/integration-test-reports
name: test-reports
Copy link
Contributor

Choose a reason for hiding this comment

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

codecov gives us test coverage but not test results

@@ -37,6 +37,9 @@ jobs:

- name: Publish docker image
run: ./gradlew publish dockerPushImages
env:
DOCKER_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
Copy link
Contributor

Choose a reason for hiding this comment

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

write credentials should be kept distinct from read credentials (this will be a read-only token once dockerhub allows assigning permission to tokens)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants