Skip to content

Don't run all jobs on every PR#7693

Merged
mrcasals merged 4 commits intodevelopfrom
chore/dont-run-all-jobs
Mar 24, 2021
Merged

Don't run all jobs on every PR#7693
mrcasals merged 4 commits intodevelopfrom
chore/dont-run-all-jobs

Conversation

@leio10
Copy link
Copy Markdown
Contributor

@leio10 leio10 commented Mar 23, 2021

🎩 What? Why?

The queues of Github Actions for the decidim are starting to be too big (currently, there are more than 800 jobs enqueued). This PR tries to help with that, preventing to execute some jobs when its clearly not needed.

To decide when a job needs to be executed, we take into account files changed by a PR (we are not changing anything for the develop branch nor the releases branches). For each module, we add the list of paths that should be taken into account. This list is generated automatically by the script .github/workflows/dependencies.sh based on the contents of the .gemspec file of each module.

At the moment, the workflow files should be updated manually. In the future, we could work on a script that updates the .yml automatically before each commit or that check that on the main job.

This PR also updates the docs to explain this improvement.

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@leio10 leio10 requested review from a team, andreslucena and mrcasals March 23, 2021 16:15
mrcasals
mrcasals reviewed Mar 24, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Thank you so much, @leio10!

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Mar 24, 2021

The idea behind this is that we're running all CI jobs once a PR is merged. But when the PR is open, we can reduce the amount of tests that are executed.

This was our first idea:

  1. Get the list of modified files in this PR
  2. From it, get the list of modules related to those files. That’d usually be something like getting the first folder in the file path.
  3. Build a tree of dependencies by checking the gemspec of each module. Eg. proposal depends on core, admin and N other modules
  4. Get the list of modules depending on a given module (eg everyone depends on core, nobody depends on decidim-pages or decidim-elections)
  5. Run the tests ONLY for the modules directly affected by changes, and for those that depend on those modules.

This is a little naive, but should work.

Using GitHub Actions, we can improve the original idea by using a paths filter to only run a job if any of those paths are affected. This means we can get a list of internal decidim dependencies for each module (using the .github/workflows/dependencies.sh file), generate the paths list for each module and update the workflow.

In reality, this means that if we modify decidim-core it will trigger the CI for all modules, but if we only modify decidim-elections then we can just run the CI for that one module, because no other Decidim module depends on it.

This will reduce the amount of jobs to be enqueued per PR.

Crowdin PRs are not affected and will still skip the CI jobs.

@mrcasals mrcasals merged commit dc3f989 into develop Mar 24, 2021
@mrcasals mrcasals deleted the chore/dont-run-all-jobs branch March 24, 2021 07:40
This was referenced Mar 24, 2021
Closed
Closed
entantoencuanto added a commit that referenced this pull request Mar 26, 2021
@entantoencuanto
Merge branch 'develop' into chore/rails-6-upgrade
f69e5a4 
* develop: (64 commits)
  Fix report mailers when author is a meeting (#7683)
  New Crowdin updates (#7729)
  Fix form builder assuming proposals module availability (#7689)
  Fix a series of issues with proposal attachments in the public area (#7699)
  New Crowdin updates (#7711)
  Add accessibility labels to the <nav> menus (#7709)
  Fix heading order on the home page (#7710)
  Fix dropdown menu accessibility audits (#7708)
  Fix the aria attribute names (no aria prefix) (#7707)
  Fix validations for registration related fields in Conference form (#7675)
  New Crowdin updates (#7613)
  Use comments counter cache instead of additional query (#7627)
  Upgrade to decidim-bulletin_board 0.15.2 (#7659)
  Bump mimemagic to 0.3.6 (#7701)
  Ensure pagination elements per page is a valid option (#7680)
  Fix link to "Getting started guide" in README.adoc (#7695)
  Fix link to CONTRIBUTING.adoc in PR template (#7696)
  Fix the screen reader class name for comments opinion toggle (#7698)
  Fix initiative-m card hashtags (#7679)
  Don't run all jobs on every PR (#7693)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreslucena andreslucena Awaiting requested review from andreslucena

1 more reviewer

@mrcasals mrcasals mrcasals approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

target: developer-experience

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@leio10 @mrcasals