Skip to content

Run CI on pull request and on push#21

Merged
mathbunnyru merged 3 commits intomasterfrom
bthomee/on
Nov 20, 2025
Merged

Run CI on pull request and on push#21
mathbunnyru merged 3 commits intomasterfrom
bthomee/on

Conversation

@bthomee
Copy link

@bthomee bthomee commented Nov 20, 2025

The on: push does not always trigger on new commits pushed to a branch. This change adds a specific on: pull_request.

@bthomee bthomee requested a review from mathbunnyru November 20, 2025 14:24
Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

At the bottom of this file, instead of having if: ${{ github.ref_type == 'branch' && github.ref_name == github.event.repository.default_branch }} let's write it much simpler: if: ${{ github.event_name == 'push' }}

Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

Please, see my comment above.

Also, please make it work for forks - you shouldn't login, so it should also be under if

@bthomee
Copy link
Author

bthomee commented Nov 20, 2025

Please, see my comment above.

Also, please make it work for forks - you shouldn't login, so it should also be under if

Good call, done.

@bthomee
Copy link
Author

bthomee commented Nov 20, 2025

Please, see my comment above.
Also, please make it work for forks - you shouldn't login, so it should also be under if

Good call, done.

Regarding forks... let me add the same logic here as we have in the rippled repo.

@mathbunnyru mathbunnyru merged commit 177514d into master Nov 20, 2025
1 check passed
@mathbunnyru mathbunnyru deleted the bthomee/on branch November 20, 2025 15:10
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.

2 participants