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

Translate build and test workflow to Kotlin DSL #106

Merged

Conversation

krzema12
Copy link
Owner

@krzema12 krzema12 commented Jan 19, 2022

Part of typesafegithub/github-workflows-kt#3.

The workflows in kotlin-python are the main inspiration to me to create https://github.com/krzema12/github-actions-kotlin-dsl/.
That's why I'd like to use it in this project, so that we have a real-life scenario and "customer zero".

I'm migrating just one workflow as an experiment, and once it's merged, I'll do the same with the other one. By having them in the same Kotlin file or sharing common Kotlin file, we'll manage to remove some across-workflows duplication.

Testing

I compared workflows in runtime, before and after this change:

I also checked how it works in case of forgetting to regenerate the YAML. It now fails at runtime:

Screenshot from 2022-01-19 12-25-59

and in the future we can e.g. add auto-generated job that regenerates the YAML. The golden grail is GitHub supporting such Kotlin DSL, without any (semi)manual generation steps.

@krzema12 krzema12 requested a review from SerVB January 19, 2022 13:51
@krzema12 krzema12 force-pushed the translate-build-and-test-workflow-to-Kotlin-DSL branch from b04984e to 5293ea3 Compare January 23, 2022 20:55
@krzema12 krzema12 marked this pull request as draft January 23, 2022 20:55
@krzema12 krzema12 force-pushed the translate-build-and-test-workflow-to-Kotlin-DSL branch from 5293ea3 to cad2502 Compare January 24, 2022 07:56
@krzema12 krzema12 marked this pull request as ready for review January 24, 2022 14:57
Copy link
Collaborator

@SerVB SerVB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what about another workflow file?

My though regarding deduplication was quite opposite: merge to the single file. However, that would include some error-prone conditions based on the file input (like workflow_dispatch or push). You way is better that we can have linear workflows. And I generally love the DSL :)

Comment on lines +91 to +96
run: |
python/experiments/generate-box-tests-reports.main.kts \
--test-task=${{ matrix.testTask }} \
--failed-tests-report-path=failed-tests.txt \
--box-tests-report-path=box-tests-report.tsv \
--failure-count-report-path=failure-count.tsv
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I was OK with long lines ;)

Copy link
Owner Author

@krzema12 krzema12 Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually found them hard to work with. Horizontal scrolling is painful :)

@krzema12
Copy link
Owner Author

And what about another workflow file?

I'll tackle it in a separate PR, will experiment with having both in one file VS separated :)

@krzema12 krzema12 merged commit 00b3b60 into python-backend Jan 26, 2022
@krzema12 krzema12 deleted the translate-build-and-test-workflow-to-Kotlin-DSL branch January 26, 2022 19:02
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.

None yet

2 participants