-
Notifications
You must be signed in to change notification settings - Fork 17
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
chore: use matrix for the CI APIC-255 #75
Conversation
b4c0587
to
8339cee
Compare
c7606de
to
50254f9
Compare
89b82e4
to
8eb4776
Compare
de541c0
to
c7a7bef
Compare
a1a640c
to
893ec6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvements! That's a lot of redundant code removed
I'll do an other pass when the specs script is done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really niiiiiiiiiice
if [[ $matrix == '{"client":["no-run"]}' ]]; then | ||
run="false" | ||
else | ||
run="true" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part could be done in the scripts, no?
I think we should reduce the logic in the run
as it's easier to debug/write in the scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't get it to work with github actions, I cannot split the output properly with any command, maybe bash is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking again, it would impact adding jq
logic to handle double returns, you can skip this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't get it to work with github actions, I cannot split the output properly with any command, maybe bash is different.
My guess would be to return a json object with run
and matrix
and split the result with jq
to set the output, but not sure if it's worth compared to a simple if else
else | ||
empty | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jq
forces empty else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't work without it so I guess so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good to know, maybe acts as ternary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go! Let's see that in action
GG
🧭 What and Why
🎟 JIRA Ticket: APIC-255
Because we will have 7 clients times at least 11 languages, we don't want to write the same job and same checks for all 77 tests, we can factorise everything using matrix.
In this first PR each languages have it's own job, but they can all be under the same matrix, unless we want a setup job for each languages.
The source of truth is the file
openapitools.json
, the same way as for the scripts and the CTS, this way we don't have to do anything when adding a new client.The cache action is not done yet, i'm not sure if we can combine composite and matrix.
Changes included:
🧪 Test
CI