Skipped test building of docker image on push

[bmispelon]

The image will actually get built (and published) anyway.

[bmispelon]

@tobiasmcnulty does that make sense? Or am I missing a usecase for testing the build before actually publishing it?

[tobiasmcnulty]

@bmispelon Good catch. I agree it would be nice to remove the duplicate build of the Docker image with the production/deployed requirements file.

This change would no longer test the Docker build with test requirements after a PR is merged (relevant only if the offending PR was not updated to latest main before merging). That may be an edge case not worth worrying about.

In the future, maybe the two files could be consolidated to eliminate the duplicate build, but still always test both builds on pushes to main?

[bmispelon]

This change would no longer test the Docker build with test requirements after a PR is merged (relevant only if the offending PR was not updated to latest main before merging). That may be an edge case not worth worrying about.

Ah right, good point. That does seem to be quite the edge case, plus as it turns out our instructions for testing in docker haven't been working for a while (#1817 )

In the future, maybe the two files could be consolidated to eliminate the duplicate build, but still always test both builds on pushes to main?

That's an interesting idea: test the build with the tests.txt requirements then build+publish the one with prods.txt? I could file a new ticket for that.

[tobiasmcnulty]

This change would no longer test the Docker build with test requirements after a PR is merged (relevant only if the offending PR was not updated to latest main before merging). That may be an edge case not worth worrying about.

Ah right, good point. That does seem to be quite the edge case, plus as it turns out our instructions for testing in docker haven't been working for a while (#1817 )

Interesting, maybe we could test this flow in CI, too. I vaguely recall thinking about that at DjangoCon and then running out of time and/or steam. 😅

In the future, maybe the two files could be consolidated to eliminate the duplicate build, but still always test both builds on pushes to main?

That's an interesting idea: test the build with the tests.txt requirements then build+publish the one with prods.txt? I could file a new ticket for that.

That should work. I think it could still be a matrix build so they both happen at the same time.

Another option would be to keep the workflow files separate (maybe better if we want CI to run tests in Docker too?), but add a variable to push only if the branch is main.

Anyways, I think this is good, thanks! 👍🏻