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

Change GitHub Actions Workflow Triggers #127

Merged
merged 3 commits into from
Jul 7, 2021

Conversation

AlexandruGG
Copy link
Collaborator

This PR looks to solve workflows triggering multiple times on PRs.

@@ -2,7 +2,6 @@

on:
- pull_request
- push
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@drupol do you want these to run in master as well, after a merge?

Currently we have them set up like that but I don't know if it was intentional or not: https://github.com/loophp/collection/actions?query=branch%3Amaster

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's necessary. All the changes happening in this project should be committed through PRs and not directly in the master branch, so... it will be tested in PRs. WDYT ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2021-07-07 at 09 22 07

I think some of the badges on the readme depend on that 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough, let's not modify these.

Comment on lines 4 to 7
push:
branches:
- master
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.

I decided to keep it on the safe side so this means it runs:

  • on pull requests
  • on pushes but only to the master branch (effectively after merging a PR as well)

Copy link
Collaborator

@drupol drupol left a comment

Choose a reason for hiding this comment

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

The indentation has been changed... and it was not supposed to.

Based on the .editorconfig, the indentation is 4 spaces.

If you're using VSCode or VSCodium with the .editorconfig plugin, do: SHIFT+CTRL+i to reformat the file correctly.

@AlexandruGG
Copy link
Collaborator Author

The indentation has been changed... and it was not supposed to.

Based on the .editorconfig, the indentation is 4 spaces.

If you're using VSCode or VSCodium with the .editorconfig plugin, do: SHIFT+CTRL+i to reformat the file correctly.

Nice, thanks for that I wasn't using the .editorconfig plugin but I got it now 😁 . Formatted with it now.

@AlexandruGG AlexandruGG marked this pull request as ready for review July 7, 2021 08:41
@drupol drupol merged commit 36d1272 into master Jul 7, 2021
@drupol drupol deleted the feature/change-workflow-triggers branch July 7, 2021 08:52
@drupol
Copy link
Collaborator

drupol commented Jul 7, 2021

Merci :)

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.

None yet

2 participants