Skip to content

ci: migrating to GitHub actions - #47

Merged
JBAhire merged 7 commits into
mainfrom
gha-ci
Jan 6, 2021
Merged

JBAhire merged 7 commits into
mainfrom
gha-ci

Conversation

@JBAhire

@JBAhire JBAhire commented Dec 29, 2020

Copy link
Copy Markdown
Member

Description

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

@aaron-steinfeld aaron-steinfeld left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread .github/workflows/pr-build.yml
Comment thread .github/workflows/pr-build.yml Outdated
Comment thread .github/workflows/pr-build.yml Outdated
- 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@aaron-steinfeld aaron-steinfeld Jan 3, 2021 •

Copy link
Copy Markdown
Contributor

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.

Comment thread .github/workflows/pr-test.yml Outdated
gradle-packages-${{ runner.os }}-${{ github.job }}
gradle-packages-${{ runner.os }}

- name: Unit test

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, nope :)

@JBAhire

JBAhire commented Jan 4, 2021

Copy link
Copy Markdown
Member Author

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

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

Copy link
Copy Markdown
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

@aaron-steinfeld aaron-steinfeld left a comment

Copy link
Copy Markdown
Contributor

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

github-actions Bot commented Jan 6, 2021

Copy link
Copy Markdown

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