On Tue, Oct 31, 2023 at 08:46:28AM +0100, Patrick Steinhardt wrote: > On Mon, Oct 30, 2023 at 11:46:44AM -0400, Taylor Blau wrote: > > On Thu, Oct 26, 2023 at 09:59:59AM +0200, Patrick Steinhardt wrote: > > > And this is exactly what this patch series does: it adds GitLab-specific > > > knowledge to our CI scripts and adds a CI definition that builds on top > > > of those scripts. This is rather straight forward, as the scripts > > > already know to discern Azure Pipelines and GitHub Actions, and adding > > > a third item to this list feels quite natural. And by building on top of > > > the preexisting infra, the actual ".gitlab-ci.yml" is really quite > > > small. > > > > > > I acknowledge that the Git project may not be willing to fully support > > > GitLab CI, and that's fine with me. If we want to further stress that > > > point then I'd also be perfectly happy to move the definitions into the > > > "contrib/" directory -- it would still be a huge win for our workflow. > > > In any case, I'm happy to keep on maintaining the intgeration with > > > GitLab CI, and if things break I'll do my best to fix them fast. > > > > I don't have any strong opinions here, but my preference would probably > > be to keep any GitLab-specific CI configuration limited to "contrib", if > > it lands in the tree at all. > > As mentioned, I would not mind at all if we wanted to instead carry this > as part of "contrib/". I think my concern with having it in Junio's tree at all is that it gives the impression that this is being maintained indefinitely. There is definitely some cruft in contrib that we should probably get rid of. But since this is coming from an organization that sponsors work on the Git project, I think that my concerns there are somewhat more relaxed. I'm not worried about this going unmaintained, so I have no objection to having it live in contrib. > Yup, that's a valid concern. As mentioned, this patch series does not > have the intent to make GitLab CI a second authoritative CI platform. > GitHub Actions should remain the source of truth of whether a pipeline > passes or not. Most importantly, I do not want to require the maintainer > to now watch both pipelines on GitHub and GitLab. This might be another > indicator that the pipeline should rather be in "contrib/", so that > people don't start to treat it as authoritative. Yeah, I agree with everything you said here. > > My other concern is that we're doubling the cost of any new changes to > > our CI definition. Perhaps this is more of an academic concern, but I > > think my fear would be that one or the other would fall behind on in > > implementation leading to further divergence between the two. > > > > I think having the new CI definition live in "contrib" somewhat > > addresses the "which CI is authoritative?" problem, but that it doesn't > > address the "we have two of these" problem. > > I do see that this requires us to be a bit more careful with future > changes to our CI definitions. But I think the additional work that this > creates is really very limited. Except for the `.gitlab-ci.yml`, there > are only 54 lines specific to GitLab in our CI scripts now, which I > think should be rather manageable. Agreed. > I also think that it is sensible to ensure that our CI scripts are as > agnostic to the CI platform as possible, as it ensures that we continue > to be agile here in the future if we are ever forced to switch due to > whatever reason. In the best case, our CI scripts would allow a user to > also easily run the tests locally via e.g. Docker. We're not there yet, > but this patch series is a good step into that direction already. Agreed there as well. > Last but not least, I actually think that having multiple supported CI > platforms also has the benefit that people can more readily set it up > for themselves. In theory, this has the potential to broaden the set of > people willing to contribute to our `ci/` scripts, which would in the > end also benefit GitHub Actions. > > In my opinion, this benefit is demonstrated by this patch series > already: besides all the changes that aim to prepare for GitLab CI, > there are also patches that deduplicate code and improve test coverage > for Alpine Linux. These changes likely wouldn't have happened if it > wasn't for the GitLab CI. Yeah, I appreciate your careful and patient explanation here. I think that my concerns here are resolved. Thanks, Taylor
