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

One branch to rule them all #169

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

andrewnicols
Copy link
Contributor

@andrewnicols andrewnicols commented Mar 18, 2023

This PR is an attempt to have on e branch which contains all versions and all codebases. Although this does lead to duplicate code, it may be easier to maintain.

Pushing as a test to see how it plays out at the moment.

@andrewnicols andrewnicols changed the title Onebranchtorulethem One branch to rule them all Mar 18, 2023
@andrewnicols andrewnicols force-pushed the onebranchtorulethem branch 3 times, most recently from b2c975a to ae49f19 Compare March 18, 2023 07:02
@andrewnicols
Copy link
Contributor Author

This is working more-or-less as I was hoping for and it will greatly simplify our workflow. One PR can now target all relevant images.

There is a little duplication in that each file is a copy of the previous branch, but that's true of a multi-branch approach (and harder to compare the files too).

We could possibly create a wrapper and generator, which is what I originally tried to do in #44, but really... we have 6-12 images total at any one time, and they differ in ways that make it quite hard to reduce that duplication without a whole heap of boilerplate.

@andrewnicols
Copy link
Contributor Author

Ah, the only thing that I don't have solved is badges for the README.md.

GHA badges only work on a once-per-workflow basis, and do not generate one per matrix combination.

@andrewnicols andrewnicols marked this pull request as ready for review March 18, 2023 07:33
@andrewnicols
Copy link
Contributor Author

Oh, and the push is currently not setting the right value for the pushed image name.
Need to workout how to get the raw value to show either ${{matrix.phpversion}}/${{matrix.osversion}} or dev

@stronk7
Copy link
Member

stronk7 commented Mar 18, 2023

Side comment: I must confess I'm not entirely convinced about the benefits of this change. I've been thinking about it (move everything to a single branch) for some long time, and always end with the conclusion about separate branches being better.

It's super easy to backport anything, everything is fully separated, in case there are tiny lib differences / details... each branch its image, push and build individually... and with the all-in-one we lose many of those benefits.

Just IMO, ciao :-)

@stronk7
Copy link
Member

stronk7 commented Aug 6, 2023

Hi,

what do you think if we close this? I've just backported #173 (for example) to all branches >= 7.4 and really it's super easy (easier than having everything in the same branch, honestly).

Ciao :-)

@stronk7 stronk7 force-pushed the master branch 3 times, most recently from 64fcd51 to 057db2d Compare August 13, 2023 14:46
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