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

ci: no pipeline on pull_request #2016

Merged
merged 1 commit into from
Jan 19, 2022
Merged

ci: no pipeline on pull_request #2016

merged 1 commit into from
Jan 19, 2022

Conversation

gsemet
Copy link
Member

@gsemet gsemet commented Jan 19, 2022

No description provided.

@mlouielu
Copy link
Collaborator

I believe we still need pull_request for other contributor, we can reduce it to build with push, pull_request, and release_test with push only.

@gsemet
Copy link
Member Author

gsemet commented Jan 19, 2022

I do not see the difference between them? If the checks has been done on the push of the branch, why redoing them after each even on the pull request? It would make sense if we would retrieve something from the PL and check it, but we do not do that

@Davidy22
Copy link
Collaborator

Davidy22 commented Jan 19, 2022

CI on pull request is there so that we can see at a glance if someone is trying to make a pull request that breaks things, like editing old release notes files in a way that breaks release note generation. CI on push catches commits that can happen via avenues other than PR, allows people to see if their commits will pass our CI before creating their pull request, and lights the badge up. Neither are considerable for removal.

@Davidy22 Davidy22 closed this Jan 19, 2022
@gsemet
Copy link
Member Author

gsemet commented Jan 19, 2022

Sorry but I do not agree. If you look in this PL, there is only checks on "push" event, and it does the protection like you say. It is the same jobs, executed twice so it is a waste of resource.

Before:

Screenshot 2022-01-19 at 11 00 19

After:

Screenshot 2022-01-19 at 10 59 57

"pull_request" only react to event on the pull requests, like comments, review, etc. There is no use here, since we do not consume anything from it in the pipeline. For exemple, in my job we use Gitlab, we do that, we actually retrieve something from the merge request API (checklist status, review even some comments in the MR,...) and we have a job that can read this information and process it and fail or not the build for instance.

For guake, we do not use it. Events on pull request does not change the commits. If there is any review and change requested, the developers will do another git push and so another "push" event is generated and so a new pipeline is started.

Reopening.

@gsemet gsemet reopened this Jan 19, 2022
Copy link
Collaborator

@Davidy22 Davidy22 left a comment

Choose a reason for hiding this comment

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

Oh hum, I always just figured doubled up entries was how it looked. It seems like github is still optimising it down to one run each in the job report, but trimming the display'll be nice.

@gsemet gsemet merged commit 517745d into master Jan 19, 2022
@gsemet
Copy link
Member Author

gsemet commented Jan 19, 2022

thx for approval. I merge

@Davidy22 Davidy22 deleted the gsemet-patch-3 branch May 12, 2023 02:57
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.

3 participants