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

Do not run clone step if no pipeline step will run #877

Merged
merged 14 commits into from
May 18, 2022

Conversation

qwerty287
Copy link
Contributor

@qwerty287 qwerty287 commented Apr 9, 2022

Skip the clone step and ignore hook/pipeline if no pipeline step except clone would run. The status reported back to the forge is success.

Closes #778

@anbraten
Copy link
Member

anbraten commented Apr 9, 2022

With one pipeline, an issue occurs: the time says not started yet because the duration is 0 which results in displaying this message. I don't know a simple way to fix it. Also, the large white "box" next to the pipeline list looks a bit odd.

I improved the "white box" a bit in #846. Maybe that adjustment would be helpful for you in this PR as well.

What do you think about hiding empty pipelines and maybe the complete "build" in case they are empty as we are doing a similar thing for pipelines steps which are not executed => they are not shown to the user as well?

@qwerty287
Copy link
Contributor Author

Yeah I thought about this too but wasn't able to implement it after some experiments (I don't know any web frontend dev, which I'd need to add such a weird v-if statement to the vue files, sorry). Hiding the complete build would be possible, then an easy solution would also to just return if coming from the hook (i.e. don't initiate a build). This wouldn't report a status back to the forge, which can be problematic if using pull requests that require the checks. Hinding it only from the list is a good idea, but would require more effort which I'm not able to do because I don't understand woodpecker's structure enough (and don't vue in general).

@6543 6543 added the feature add new functionality label May 12, 2022
@6543 6543 added this to the 1.0.0 milestone May 12, 2022
@6543
Copy link
Member

6543 commented May 12, 2022

I would: if there are no steps, dont add clone but report success back

and only if global pipeline filter #when (#283 ) do filter out make no response

agent/runner.go Outdated Show resolved Hide resolved
@qwerty287
Copy link
Contributor Author

After the commits I pushed now it just skips and ignores the hook. Should it send a "success" status back and should this always happen for empty builds?

@codecov-commenter
Copy link

codecov-commenter commented May 13, 2022

Codecov Report

Merging #877 (0d0a85c) into master (0b2b776) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

❗ Current head 0d0a85c differs from pull request most recent head bcb282c. Consider uploading reports for the commit bcb282c to get more accurate results

@@            Coverage Diff             @@
##           master     #877      +/-   ##
==========================================
- Coverage   51.63%   51.53%   -0.11%     
==========================================
  Files          79       79              
  Lines        6075     6074       -1     
==========================================
- Hits         3137     3130       -7     
- Misses       2756     2762       +6     
  Partials      182      182              
Impacted Files Coverage Δ
server/shared/procBuilder.go 85.76% <100.00%> (-0.43%) ⬇️
server/logging/log.go 48.83% <0.00%> (-5.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b2b776...bcb282c. Read the comment docs.

@6543
Copy link
Member

6543 commented May 14, 2022

After the commits I pushed now it just skips and ignores the hook. Should it send a "success" status back and should this always happen for empty builds?

If no steps but pipeline is not filtered golbaly -> success
if pipline is filtered globaly -> none

(that way you can still set some pipeline X as required for a pull request, even if a pull dont touch part X of the codebase AND also just define dont report back if)

@qwerty287
Copy link
Contributor Author

Done.

@6543 6543 self-requested a review May 17, 2022 16:01
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

some todos ... but that should be covered in another pulls (refactoring etc ...)

@6543 6543 enabled auto-merge (squash) May 18, 2022 21:12
@6543 6543 merged commit f05f918 into woodpecker-ci:master May 18, 2022
@qwerty287 qwerty287 deleted the no-clone branch May 19, 2022 16:27
6543 added a commit that referenced this pull request May 20, 2022
@6543 6543 removed this from the 1.0.0 milestone Jun 8, 2023
@6543 6543 removed the feature add new functionality label Jun 8, 2023
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.

Do not run clone step if no pipeline step will run
4 participants