-
Notifications
You must be signed in to change notification settings - Fork 86
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: Use binstall, Always run tests/linux #282
base: main
Are you sure you want to change the base?
Changes from all commits
190cfcd
9640e4d
f192918
fab3fc6
8e9d812
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,6 @@ name: Release binaries for Linux and Docker | |
|
||
on: | ||
push: | ||
branches: | ||
- main | ||
tags: | ||
- "v*.*.*" | ||
|
||
jobs: | ||
build-docker-cross: | ||
|
@@ -16,6 +12,7 @@ jobs: | |
platform: [linux/arm/v7, linux/amd64, linux/arm64] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v3 | ||
id: builder1 | ||
|
@@ -94,7 +91,7 @@ jobs: | |
uses: docker/metadata-action@v5 | ||
with: | ||
images: | | ||
ikatson/rqbit | ||
${{ vars.DOCKERHUB_USERNAME }}/rqbit, enable=${{ vars.DOCKERHUB_USERNAME != '' }} | ||
tags: | | ||
type=ref,event=branch | ||
type=semver,pattern={{version}} | ||
|
@@ -119,12 +116,24 @@ jobs: | |
uses: docker/setup-buildx-action@v3 | ||
|
||
- name: Login to Docker Hub | ||
if: "${{ vars.DOCKERHUB_USERNAME != '' }}" | ||
uses: docker/login-action@v3 | ||
with: | ||
username: ${{ vars.DOCKERHUB_USERNAME }} | ||
password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
||
- name: Build | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why split this up? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its only split up since i cant think of an easy way to keep them combined that accomplishes the end goal. I'll play around with this and see if we can keep this as a single piece. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Converting to draft until i can figure out a good way to merge them. |
||
if: "${{ vars.DOCKERHUB_USERNAME == '' }}" | ||
uses: docker/build-push-action@v6 | ||
with: | ||
push: false | ||
tags: rqbit:action | ||
platforms: linux/amd64,linux/arm64,linux/arm/v7 | ||
context: target/cross/ | ||
file: docker/Dockerfile | ||
|
||
- name: Build and push | ||
if: "${{ vars.DOCKERHUB_USERNAME != '' }}" | ||
uses: docker/build-push-action@v6 | ||
with: | ||
push: true | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this building / wasting CPU / resources on every single push to random branches
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with limiting it to branches is now it wont run when others are developing. When you work off a fork you aren't supposed to do any work in main branch so you are supposed to create a feature branch that all your work is in that you then send the PR for. If we limit it to main branch then the new developer wouldn't know that something they changed is affecting the build process until they submit the PR. Ideally they would catch all the locally but then again that would defeat the purpose of the CI to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I thought I commented on the test.yaml change. I'll expand in another comment