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

ci: fix main workflow #72

Merged
merged 2 commits into from
Mar 4, 2024
+4 −1
Merged

ci: fix main workflow #72

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e28e86d
ci: fix main workflow
nealrichardson Mar 4, 2024
96cf05a
Update .github/workflows/main.yaml
nealrichardson Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ on:
push:
branches:
- main
pull_request:
paths:
- '.github/workflows/cpp.yml'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to commit this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤦 Yes I did, but I meant to edit it to be main.yaml instead, which would explain why it didn't run here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
pull_request:
paths:
- '.github/workflows/cpp.yml'
pull_request:
paths:
- '.github/workflows/main.yaml'

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh neat! So, to confirm my understanding... this runs main.yaml anytime the file is changed on a pull request?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct--you can see that it just ran now that I fixed it. So it still wouldn't have caught the change in #60 since I didn't touch this file. But if we ever edit this file in the future, the PR will run it, we won't have to wait for it to land on main.

Since this is basically testing the Makefile, we could also have it run when the Makefile is modified?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense. I like the idea of workflows being task-focused instead of branch-focused. e.g., makefile.yaml instead of main.yaml or something...

jobs:
default:
runs-on: ubuntu-latest
Expand All @@ -21,4 +24,3 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
- run: make
- run: make install
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ PYTHON := python3

default:
make deps
make install
make test
make lint
make build
Expand Down
Loading