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.

Sign up for GitHub

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

Jump to bottom

Moved to Github Actions from drone CI #41

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Moved to Github Actions from drone CI #41

merged 1 commit into from
Oct 8, 2020

Conversation

parauliya
Copy link
Contributor

Signed-off-by: parauliya [email protected]

Description

This chnage will move the CI from drone to Github Actions

Why is this needed

Fixes: #39

How Has This Been Tested?

All checks are being passed while raising a PR

How are existing users impacted? What migration steps/scripts do we need?

No impact.

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@parauliya parauliya requested a review from a team October 5, 2020 09:05
@codecov
Copy link

codecov bot commented Oct 5, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@6ae6cc2). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #41   +/-   ##
=========================================
  Coverage          ?   23.95%           
=========================================
  Files             ?        5           
  Lines             ?      501           
  Branches          ?        0           
=========================================
  Hits              ?      120           
  Misses            ?      359           
  Partials          ?       22

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ae6cc2...7000b10. Read the comment docs.

@parauliya parauliya requested a review from gianarb October 5, 2020 09:09
@parauliya parauliya self-assigned this Oct 5, 2020
@parauliya parauliya added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 5, 2020
mmlb
mmlb suggested changes Oct 5, 2020
View reviewed changes
.github/workflows/ci.yaml Outdated
path: ./hegel
docker-images:
env:
CGO_ENABLED: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be done in the validation part right? This isn't really doing anything here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right. This will be removed.

.github/workflows/ci.yaml Show resolved Hide resolved
@parauliya parauliya force-pushed the Fix_39 branch 2 times, most recently from ce8c92d to 328b540 Compare October 6, 2020 07:15
@parauliya parauliya requested a review from mmlb October 6, 2020 07:22
gianarb
gianarb previously approved these changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@gianarb gianarb left a comment

Choose a reason for hiding this comment

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

It looks very good to me! Let's wait for @mmlb because he knows a bit better Hegel, but I can't see reasons to hold this further.

@gianarb gianarb added the ready-to-merge Signal to Mergify to merge the PR. label Oct 6, 2020
Cbkhare
Cbkhare previously approved these changes Oct 6, 2020
View reviewed changes
mmlb
mmlb suggested changes Oct 6, 2020
View reviewed changes
.github/workflows/ci.yaml Outdated
with:
go-version: '1.14.6'
- name: go fmt
run: go get golang.org/x/tools/cmd/goimports; goimports -d . | (! grep .)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be &&. I think at one point there was a typo or something and the get failed but the command continued because of ; and goimports failed but because there was no pipefail it didn't error and ! grep worked out fine. Not in this repo I think, maybe another one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@parauliya parauliya dismissed stale reviews from Cbkhare and gianarb via 7000b10 October 7, 2020 06:10
@parauliya parauliya requested review from mmlb, gianarb and Cbkhare October 7, 2020 06:39
@gauravgahlot gauravgahlot removed their request for review October 7, 2020 11:22
mmlb
mmlb approved these changes Oct 7, 2020
View reviewed changes
Copy link
Contributor

@mmlb mmlb left a comment

Choose a reason for hiding this comment

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

Pretty sure we're going to remove the pull_request bit pushing to quay since secrets aren't provided for PRs. But we can do that in a follow up PR.

@parauliya parauliya added ready-to-merge Signal to Mergify to merge the PR. and removed ready-to-merge Signal to Mergify to merge the PR. labels Oct 8, 2020
@gianarb gianarb merged commit 1c9e1e1 into master Oct 8, 2020
@gianarb gianarb deleted the Fix_39 branch October 8, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmlb mmlb mmlb approved these changes

@gianarb gianarb gianarb approved these changes

@Cbkhare Cbkhare Cbkhare approved these changes

Assignees

@parauliya parauliya

Labels
kind/feature Categorizes issue or PR as related to a new feature. ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate to GitHub Actions
4 participants
@parauliya @mmlb @gianarb @Cbkhare