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

Fixes/lack of ci #8

Merged
merged 16 commits into from Oct 30, 2020
Merged

Fixes/lack of ci #8

merged 16 commits into from Oct 30, 2020

Conversation

etagwerker
Copy link
Member

Hey,

This PR enables CI for this project. Also, it fixes #2 and fixes #7.

Please check it out.

Thanks!

@etagwerker etagwerker added the wip label Oct 27, 2020
@cleicar
Copy link

cleicar commented Oct 28, 2020

Hey @etagwerker did you notice that CI is failing here?

@etagwerker
Copy link
Member Author

@cleicar yeah, I had noticed and that is why I added the wip label 😄

CI is working now and I've created an issue to fix the failure with skunk's action: #9

@@ -0,0 +1,54 @@
name: Skunk
on: [push]
Copy link
Contributor

@bronzdoc bronzdoc Oct 29, 2020

Choose a reason for hiding this comment

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

We want to also add the pull_request event so CI status is reflected in the PR when you open a new PR

on:[push, pull_request]

# .github/workflows/ci.yml

name: CI
on: [push]
Copy link
Contributor

Choose a reason for hiding this comment

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

pull_request too

@@ -0,0 +1,89 @@
# PostgreSQL. Versions 9.3 and up are supported.
Copy link
Contributor

@bronzdoc bronzdoc Oct 29, 2020

Choose a reason for hiding this comment

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

just curious why this file ends with .github

Copy link
Member Author

Choose a reason for hiding this comment

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

@bronzdoc This file is getting used within the GitHub Actions only, so that is why it has that suffix. It shouldn't be used for anything else.

@etagwerker
Copy link
Member Author

@bronzdoc Just addressed your comments 👍

@etagwerker
Copy link
Member Author

@bronzdoc I don't think it is a great idea to make the events push and pull_request. It seems to run duplicated jobs, which is going to consume more resources than we need. 😕

Screen Shot 2020-10-29 at 10 24 02 PM

@bronzdoc
Copy link
Contributor

@etagwerker I gave this a look and I think what we want is:

on:
  push:
    branches:
    - main
  pull_request:
    branches:
    - main

https://github.community/t/how-to-trigger-an-action-on-push-or-pull-request-but-not-both/16662

What you think?

@etagwerker
Copy link
Member Author

@bronzdoc That looks great. Thanks!

@bronzdoc
Copy link
Contributor

@etagwerker it seems there is still an issue in the build, can you have a look?

@etagwerker
Copy link
Member Author

@bronzdoc Are you talking about the Skunk action failure? I added a comment about that over here: #8 (comment)

I see that CI works fine though and this PR was about CI (I meant test suite) not Skunk's GitHub action.

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

Successfully merging this pull request may close these issues.

CI missing for this project rake should default to rake test and run the test suite
3 participants