Skip pipeline if config not found #2313
Conversation
|
Don't know it that's the best way to achieve this, but it seems to be working. |
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2313 +/- ##
=======================================
Coverage ? 40.42%
=======================================
Files ? 183
Lines ? 11035
Branches ? 0
=======================================
Hits ? 4461
Misses ? 6219
Partials ? 355 ☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here. |
|
Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-2313.surge.sh |
|
Could you add a note to the migration docs. Otherwise lgtm |
|
@anbraten Sure, done. |
|
maybe we can skip pipelines with missing config only for non-default branches? |
|
Can we please just merge it for now. We agreed in the issue already that there are multiple options to improve it. |
|
Argh... @6543 this needs an approval from you as you requested the change... |
| pipeline.Started = time.Now().Unix() | ||
| pipeline.Finished = pipeline.Started | ||
| pipeline.Status = model.StatusError | ||
| pipeline.Error = fmt.Sprintf("pipeline definition not found in %s", repo.FullName) | ||
| pipeline.Error = err.Msg | ||
| return nil, err |
There was a problem hiding this comment.
Why not just add?
| return nil, err | |
| if pipeline.Branch != repo.Branch { | |
| return nil, err | |
| } |
There was a problem hiding this comment.
Why not just let the user control it? Why should we assume that the default branch requires a CI config for every user or setup? We can add options to disable/enable this feature later right now irgnoring it and logging an error in the backend is good enough IMO.
There was a problem hiding this comment.
Not for my use case, this will increase our support team work on finding out why pipelines are not working for new projects because even with current error that config was not found there were some developers that asked for help. Now it will be even harder to find that as developers won't see anything and it will just not work silently.
You are currently taking away what I need to make it how you need. Imho my proposed solution would work for both our needs
There was a problem hiding this comment.
I understand your point but I still disagree. However not my decision, Ill leave this PR to someone else to pick up or make a decision.
There was a problem hiding this comment.
But from what you wrote previously this would work also for you so this would be good middle ground that would be good and not too breaking change and later on as you said this could be improved for other cases if needed
Fixes: #2312