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

ci: migrating to GitHub actions #47

Merged
merged 7 commits into from
Jan 6, 2021
Merged

ci: migrating to GitHub actions #47

merged 7 commits into from
Jan 6, 2021

Conversation

JBAhire
Copy link
Member

@JBAhire JBAhire commented Dec 29, 2020

Description

Migrating to GitHub actions from CircleCI as per hypertrace/hypertrace#144

Copy link
Contributor

@aaron-steinfeld aaron-steinfeld left a comment

Choose a reason for hiding this comment

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

A few thoughts on optimizations before broader rollouts

.github/workflows/pr-build.yml Show resolved Hide resolved
.github/workflows/pr-build.yml Outdated Show resolved Hide resolved
.github/workflows/pr-build.yml Outdated Show resolved Hide resolved
- name: Setup snyk
uses: snyk/actions/[email protected]
- name: Snyk test
run: snyk test --all-sub-projects --org=hypertrace --severity-threshold=low --policy-path=.snyk --configuration-matching='^runtimeClasspath$'
Copy link
Contributor

Choose a reason for hiding this comment

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

also may be worth moving this into an action at some point

Copy link
Member Author

Choose a reason for hiding this comment

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

I think, we can't move this step to action as it's already using other action. let me try this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

the setup itself would have to stay out (still crazy to me that GH doesn't support this yet, suspect it must be coming soon), but the test could be its own action right? or we could make the test run in a containerized action which means setup isn't needed

Copy link
Member Author

Choose a reason for hiding this comment

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

for this, I would wait as they said they will bring support for uses and if in near future. We would like to abstract this at that time anyways. :)

Let me know if that will be fine. We can surely move test but that's a single command so I won't bother a lot about it.

Copy link
Contributor

@aaron-steinfeld aaron-steinfeld Jan 3, 2021

Choose a reason for hiding this comment

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

Up to you. The problem I'm trying to avoid is that the argument set of this command has evolved over time, and now is mismatched in different repos depending on when their ci was set up. Capturing even the single line in an action avoids that (similar to our checksum action), but perhaps it's stable enough now that it's less of a concern and not worth the effort.

Also, I think using a docker-type action allows us to do everything together and do that abstraction now, but again may be more trouble than it's worth - not sure how complex those actions types are to set up.

.github/workflows/pr-test.yml Outdated Show resolved Hide resolved
gradle-packages-${{ runner.os }}-${{ github.job }}
gradle-packages-${{ runner.os }}

- name: Unit test
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no integration test in this repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just checked, it's not there.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, nope :)

@JBAhire
Copy link
Member Author

JBAhire commented Jan 4, 2021

hypertrace/actions#9 addresses all changes required on custom actions side.

@@ -0,0 +1,10 @@
name: 'Gradle command action'
inputs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Guessing this was here for testing since the workflows are using the common one now

aaron-steinfeld
aaron-steinfeld previously approved these changes Jan 5, 2021
Copy link
Contributor

@aaron-steinfeld aaron-steinfeld left a comment

Choose a reason for hiding this comment

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

Please remove the extra action either in this PR or a followup

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@JBAhire JBAhire merged commit 0759b48 into main Jan 6, 2021
@JBAhire JBAhire deleted the gha-ci branch January 6, 2021 03:49
@github-actions
Copy link

github-actions bot commented Jan 6, 2021

Unit Test Results

  21 files    21 suites   4s ⏱️
106 tests 106 ✔️ 0 💤 0 ❌

Results for commit 0759b48.

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.

5 participants