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] Refactor changed files integration #9643

Merged
merged 22 commits into from
Jan 16, 2025

Conversation

prastoin
Copy link
Contributor

@prastoin prastoin commented Jan 15, 2025

BEFORE

run
image

AFTER

run
image

Motivations:

  • less workflow to whitelist as blocking for PRs
  • less if condition per step
  • less cpu usage

cons:

  • quite verbose
  • need to manually sync the ci-NAME-status-check needs list to any other existing and should be dep jobs

Version migration

Migrated to the latest changed-files@45 version, getting rid of the set-output usage warnings

Tests runs:

With mutation:

Without mutation:

Notes

Linter

We should setup a yml prettier and linter for the .github/worfklows folder

Centralized ci-NAME-status-check logic

Unfortunately I couldn't achieve to either make a composite action or a reusable-workflow, as I could not access the correct layer to run the always but also acessing the needs context

@prastoin prastoin force-pushed the refactor-changed-files-github-actions branch 5 times, most recently from 5af75ff to 2332d86 Compare January 15, 2025 16:16
@prastoin
Copy link
Contributor Author

prastoin commented Jan 15, 2025

Request for comments

Would a love a feedback, positive as negative ! on the suggested implementation from there. ( Could be applied to ci-front and so on )
Any critics and suggestions are welcomed as always !
Thanks

@prastoin prastoin force-pushed the refactor-changed-files-github-actions branch from a9b0ddf to 978a502 Compare January 15, 2025 16:24
@prastoin prastoin force-pushed the refactor-changed-files-github-actions branch 2 times, most recently from a03237d to 553a932 Compare January 15, 2025 18:04
@prastoin prastoin self-assigned this Jan 16, 2025
@prastoin prastoin force-pushed the refactor-changed-files-github-actions branch from 3be57d2 to 00a4732 Compare January 16, 2025 10:17
@prastoin
Copy link
Contributor Author

prastoin commented Jan 16, 2025

@charlesBochet we should get in touch after this being merged to update the org ci statusCheck policy according to the new atomic job for each PR being ci-NAME-final

@prastoin prastoin force-pushed the refactor-changed-files-github-actions branch 3 times, most recently from a190497 to 0db332f Compare January 16, 2025 10:34
@prastoin prastoin marked this pull request as ready for review January 16, 2025 10:37
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR refactors GitHub Actions workflows by centralizing file change detection logic and improving failure handling across CI pipelines.

  • Added new .github/workflows/changed-files.yaml as a reusable workflow that handles file change detection for all CI jobs
  • Upgraded from tj-actions/changed-files@v11 to @v45 to eliminate set-output usage warnings
  • Added ci-*-status-check jobs across workflows to properly aggregate and handle job failures/cancellations
  • Simplified workflow structure by removing redundant conditional checks from individual steps and centralizing them in prerequisites jobs

8 file(s) reviewed, 11 comment(s)
Edit PR Review Bot Settings | Greptile

.github/workflows/ci-chrome-extension.yaml Show resolved Hide resolved
.github/workflows/ci-e2e.yaml Show resolved Hide resolved
.github/workflows/ci-front.yaml Show resolved Hide resolved
.github/workflows/ci-front.yaml Show resolved Hide resolved
.github/workflows/changed-files.yaml Show resolved Hide resolved
.github/workflows/ci-website.yaml Outdated Show resolved Hide resolved
.github/workflows/ci-test-docker-compose.yaml Show resolved Hide resolved
.github/workflows/ci-server.yaml Show resolved Hide resolved
.github/workflows/ci-test-docker-compose.yaml Show resolved Hide resolved
.github/workflows/ci-test-docker-compose.yaml Outdated Show resolved Hide resolved
@charlesBochet charlesBochet merged commit f8ddc02 into main Jan 16, 2025
32 checks passed
@charlesBochet charlesBochet deleted the refactor-changed-files-github-actions branch January 16, 2025 12:37
Copy link

Fails
🚫

node failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5584:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5555:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5564:7)�[39m
danger-results://tmp/danger-results-b02b5b91.json

Generated by 🚫 dangerJS against e1ed78e

Weiko pushed a commit that referenced this pull request Jan 16, 2025
…9670)

Related to #9643

Renaming `prerequisites` jobs to a more accurate `changed-files-check`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants