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

Jump to bottom

Move "skip ci" logic into global pipeline conditions #2216

Merged
merged 11 commits into from
Aug 17, 2023
Merged

Move "skip ci" logic into global pipeline conditions #2216

merged 11 commits into from
Aug 17, 2023

Conversation

6543
Copy link
Member

@6543 6543 commented Aug 16, 2023 •
edited
Loading

... and make custom errors follow std err conventions

this fix a 500 response if the whole pipeline is filtered out

@6543 6543 added refactor delete or replace old code wip labels Aug 16, 2023
xoxys
xoxys reviewed Aug 16, 2023
View reviewed changes
server/api/hook.go Outdated Show resolved Hide resolved
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Aug 16, 2023 •
edited
Loading

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-2216.surge.sh

@6543 6543 changed the title note TODOs and so write them down move "skip ci" logic into global pipeline conditions Aug 16, 2023
@6543 6543 marked this pull request as ready for review August 16, 2023 20:34
@6543 6543 removed the wip label Aug 16, 2023
@6543 6543 added this to the 1.1.0 milestone Aug 16, 2023
@6543 6543 requested a review from a team August 16, 2023 20:37
@6543 6543 enabled auto-merge (squash) August 16, 2023 22:09
@6543 6543 changed the title move "skip ci" logic into global pipeline conditions Move "skip ci" logic into global pipeline conditions Aug 16, 2023
@6543 6543 disabled auto-merge August 16, 2023 22:14
@6543
Copy link
Member Author

6543 commented Aug 16, 2023

hook gets ignored ... but it does not return a 204 to the webhook as expected ... need to have a look at it again :/

@6543
Copy link
Member Author

6543 commented Aug 16, 2023

fixed and found a bug that has to be backported 😅

@6543 6543 added bug Something isn't working backport-done indicates that this pull has been backported and removed bug Something isn't working backport-done indicates that this pull has been backported labels Aug 16, 2023
@6543 6543 mentioned this pull request Aug 16, 2023
Merged
@6543
Copy link
Member Author

6543 commented Aug 16, 2023

bugfix part was backported at #2230

@6543 6543 enabled auto-merge (squash) August 16, 2023 22:42
@6543 6543 disabled auto-merge August 16, 2023 22:45
@6543 6543 requested a review from lafriks August 16, 2023 22:51
@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@8d19d3b). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 37d4b5e differs from pull request most recent head 15977e2. Consider uploading reports for the commit 15977e2 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2216   +/-   ##
=======================================
  Coverage        ?   40.78%           
=======================================
  Files           ?      182           
  Lines           ?    10914           
  Branches        ?        0           
=======================================
  Hits            ?     4451           
  Misses          ?     6121           
  Partials        ?      342

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@6543
Copy link
Member Author

6543 commented Aug 17, 2023

@lafriks rerequested review because of to much changes :S

lafriks pushed a commit that referenced this pull request Aug 17, 2023
@6543
Return 204 not 500 on filtered pipeline (#2230)
a31752e 
the error check always failed, as the custom errors did not have the
`Is(error) bool` interface implemented

backport bugfix part of  #2216
@lafriks lafriks enabled auto-merge (squash) August 17, 2023 13:09
@lafriks lafriks merged commit a5ef372 into woodpecker-ci:main Aug 17, 2023
@6543 6543 deleted the todos branch August 17, 2023 14:20
@xoxys
Copy link
Member

xoxys commented Aug 21, 2023

@6543 this seems to be broken now? https://ci.thegeeklab.de/repos/13/pipeline/14

@xoxys
Copy link
Member

xoxys commented Aug 21, 2023

Argh.... because that is part of the global pipeline filter now which will not work if there is no pipeline config for a branch... There really need to be a way to ignore branches.

@xoxys xoxys mentioned this pull request Aug 21, 2023
Closed
5 tasks
@6543 6543 added the breaking will break existing installations if no manual action happens label Aug 22, 2023
@anbraten anbraten mentioned this pull request Sep 8, 2023
Closed
anbraten added a commit to anbraten/woodpecker that referenced this pull request Sep 11, 2023
@anbraten
Revert "Move "skip ci" logic into global pipeline conditions (woodpec…
54b590b 
…ker-ci#2216)"

This reverts commit a5ef372.
@anbraten anbraten mentioned this pull request Sep 11, 2023
Closed
@woodpecker-bot woodpecker-bot mentioned this pull request Oct 5, 2023
Closed
@anbraten anbraten mentioned this pull request Oct 9, 2023
Merged
@qwerty287 qwerty287 added skip-changelog and removed breaking will break existing installations if no manual action happens labels Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xoxys xoxys xoxys left review comments

@lafriks lafriks lafriks approved these changes

Assignees
No one assigned
Labels
refactor delete or replace old code skip-changelog
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@6543 @woodpecker-bot @codecov-commenter @xoxys @lafriks @qwerty287