search

apple / swift-openapi-generator

Generate Swift client and server code from an OpenAPI document.
https://swiftpackageindex.com/apple/swift-openapi-generator/documentation
Apache License 2.0
1.45k stars 120 forks source link

ci: Finish migration to Github Actions #604

Closed FranzBusch closed 2 months ago

FranzBusch commented 4 months ago

Motivation

I made more progress on the reusable workflows in NIO so it's even easier to adopt them.

Modification

This PR fixes up the renamed workflow references and adds unit tests and cxx-interop checks.

Result

Green GH actions CI

FranzBusch commented 4 months ago

@simonjbeaumont @czechboy0 Would appreciate some eyes on the failing format check

czechboy0 commented 4 months ago

@simonjbeaumont @czechboy0 Would appreciate some eyes on the failing format check

I suspect you want to add Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources to the ignore list for swift-format.

czechboy0 commented 4 months ago

See here: https://github.com/apple/swift-openapi-generator/blob/99859083e53912612b56b4b8713a971c20dab3ef/scripts/run-swift-format.sh#L34

czechboy0 commented 4 months ago

@FranzBusch It's still failing, seems the Generated entry isn't working, it's meant to match the word Generated anywhere in the path. The affected paths here are e.g. Examples/manual-generation-generator-cli-example/Sources/ManualGeneratorInvocationClient/Generated/Client.swift

czechboy0 commented 4 months ago

Ok, soundness is failing now:

** Running /code/scripts/check-license-headers.sh...
** ERROR: Unsupported file extension for file (exclude or update this script): .swiftformatignore

I think it just requires you to add .swiftformatignore to the ignore list in scripts/check-license-headers.sh

FranzBusch commented 4 months ago

@czechboy0 @simonjbeaumont This passes the format check now. Please take a look at the changes. I also added the integration tests and example tests here. Once we have this merged the only thing that is missing is

FranzBusch commented 4 months ago

Just realised I have to change something in the reusable workflow to get the integration tests to pass

simonjbeaumont commented 3 months ago

@FranzBusch noticed this got reopened. Should I assume it's still a WIP for now and wait for a ping from you to review?

FranzBusch commented 3 months ago

@simonjbeaumont @czechboy0 This PR is ready to review. The CI is all passing. I only had to skip the unacceptable language mode check and the shell check steps. I leave this up to you to re-enable them. The language mode depends on the old soundness CI to remove the text file with the words and the shell check depends on fixing up a few scripts. I would also encourage you to look into the examples CI and if you can reduce build times by using the same build dirs. You are currently rebuilding the same dependencies multiple times.

FranzBusch commented 3 months ago

The compatibility test is also missing but similarly to the above could you please set that up in a follow up. It would be a good exercise to see if the reusable workflows give you want you need.

czechboy0 commented 3 months ago

Looks broadly good to me, but I'll let @simonjbeaumont review more closely and sign off.

FranzBusch commented 2 months ago

@simonjbeaumont Gentle ping. I think we can merge this. It should replicate everything that Jenkins is currently providing.

simonjbeaumont commented 2 months ago

I'll take this, this week. Sorry for letting it linger.

simonjbeaumont commented 2 months ago

@FranzBusch I'm happy with this now and so is Honza. Can you do a final pass with your Github Actions hat on and then hit merge? :)

FranzBusch commented 2 months ago

I can't approve since I am the author but LGTM!