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

Only run the Docker build when relevant changes are made #408

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PeterJCLaw
Copy link
Member

@PeterJCLaw PeterJCLaw commented Nov 19, 2022

The Docker build is comparatively slow (1-2 minutes) and mostly redundant given that the majority of PRs change docs content rather than configuration or build system.

Testing: this PR does still run the docker job as it changes the workflow config, however PeterJCLaw#2 does not.

@PeterJCLaw PeterJCLaw force-pushed the faster-ci-less-docker branch 4 times, most recently from 6d7b736 to 02c31fb Compare November 19, 2022 10:42
@PeterJCLaw PeterJCLaw marked this pull request as ready for review November 19, 2022 10:43
@RealOrangeOne
Copy link
Member

I tend to find these kinds of "Only run CI when a certain file changes" quite brittle. Is the fact CI takes a little longer a problem? Generally getting a PR reviewed in under a minute is quite difficult, and now we live in a world with auto-merge, it shouldn't be too restrictive.

@PeterJCLaw
Copy link
Member Author

It's not about the review itself; it's more that when putting up a draft PR waiting for CI to go green before marking ready for review means waiting a long time.

@PeterJCLaw
Copy link
Member Author

Yes, I know I can/should run the checks locally, however if we want to encourage contributions from a wider audience then supporting the develop-in-GitHub case and making that have a fast feedback loop is worthwhile.

The Docker build is comparatively slow (1-2 minutes) and mostly
redundant given that the majority of PRs change docs content rather
than configuration or build system.
@PeterJCLaw
Copy link
Member Author

@RealOrangeOne I appreciate the concern around fragility, however looking at the time since this PR was created there has been one change that it's needed updating to accommodate and that change wasn't related to Docker (it was the renaming of master to main).

It would be great if we could ship this so that contributors aren't slowed down by this mostly redundant CI.

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.

2 participants