feat(ci): make ci push generated code #244

shortcuts · 2022-03-09T18:02:18Z

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-361

Changes included:

This PR adds a codegen GitHub action at the end of the CI process, which pushes the generated code to a generated/<branchName> branch and provide a comment for us to preview it.

Stacked PRs:

The previously generated branch is deleted and the generation runs from the last commit, creating a new generated branch for the current one.

`main`:

Same logic as stacked PRs, but pushes to a generated/main branch, we can after that use it to open a PR and push to main for example.

Other repositories

The logic would be similar to main, but push on an other repository based on the changes location.

e.g. clients/algoliasearch-client-javascript changed, we push it to algolia/algoliasearch-client-javascript next branch.

Next steps

Add more generated code to the CI (e.g. generated tests)
Fix the matrix issue which requires always(): I initially removed it here chore: make cache keys cleaner #242, but it's in fact needed when a matrix does not have elements to run.

🧪 Test

Preview on #245

netlify · 2022-03-09T18:02:31Z

✔️ Deploy Preview for api-clients-automation canceled.

🔨 Explore the source changes: 79d650f

🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/622f1d10e663db0008bd8659

scripts/release/common.ts

scripts/ci/codegen/cleanGeneratedBranch.ts

scripts/ci/codegen/pushGeneratedCode.ts

eunjae-lee

allez!

millotp

This will really make the repo look automated :)

millotp · 2022-03-14T10:00:24Z

.github/actions/cache/action.yml

@@ -40,6 +40,115 @@ runs:
      shell: bash
      run: curl -L "https://repo1.maven.org/maven2/org/openapitools/openapi-generator-cli/5.4.0/openapi-generator-cli-5.4.0.jar" > /tmp/openapi-generator-cli.jar

+    # Restore bundled specs: used during 'client' generation or pushing the 'codegen'
+    - name: Restore built abtesting spec


Why are those cached here ? I think we have duplication between bundled and dist, and if this needs to be cached I think it should be part of .gitignore too, so that we don't have to gen them ourself

Why are those cached here ?

They are cached as generated clients, for now the CI will only push bundled specs and generated clients. We shouldn't push them anymore too.

Ideally, we should have the same part for the CTS

millotp · 2022-03-14T10:02:38Z

.github/workflows/check.yml

+  codegen:
+    runs-on: ubuntu-20.04
+    timeout-minutes: 10
+    if: ${{ always() }}


Suggested change

if: ${{ always() }}

This is a no-op right ?

Unfortunately, always seems to be as good as a promise hell.

We've introduced always in our CI because the matrix does not return a failure/success status on empty matrix, removing the always here will skip the codegen (or any other job without always after a matrix step) on empty matrix.

I think I found a workaround to make the matrix return a status, but I'll try to fix it an a later PR

millotp · 2022-03-14T10:04:52Z

.github/workflows/codegen-cleanup.yml

+env:
+  CACHE_VERSION: '9'


We already have this variable in multiple files, this will lead to issues and incorrect use of cache if we don't update them at the same time, you think it's possible to have it inside a file like .cache-version, and read it ?

IIRC there's an issue with env from files in a composite action, but I can check!

millotp · 2022-03-14T10:09:36Z

scripts/ci/codegen/cleanGeneratedBranch.ts

+ *
+ * @param headRef - The name of the branch to search for.
+ */
+export async function cleanGeneratedBranch(headRef?: string): Promise<void> {


This function must not be exported, and you can remove the parameter headRef, because it's meant to be used as a standalone (if it's imported into another file it will run cleanGeneratedBranch)

(I've resolved other two comments to keep the conv here)

Since this function either runs from the script with an env variable or from a function call with a parameter, I've went with this solution.

I can also inline it like you suggested in #244 (comment) and inlining the HEAD_REF env var, I don't have a strong opinion so feel free to chose :D

The solution that would make more sense to me is having the cleanGenerateBranch.ts as a standalone script, and only accept the headRef as a cli parameter through process.argv, that way there is no surprise, wdyt ?

I'm fine with both actually, is that what you had in mind? b1d5dea

scripts/ci/codegen/pushGeneratedCode.ts

millotp · 2022-03-14T10:16:15Z

scripts/package.json

@@ -5,6 +5,8 @@
    "build": "tsc",
    "createReleaseIssue": "yarn build && node dist/scripts/release/create-release-issue.js",
    "processRelease": "yarn build && node dist/scripts/release/process-release.js",
+    "pushGeneratedCode": "yarn build && node dist/scripts/ci/codegen/pushGeneratedCode.js",


In general if the code needs to be build everytime it's better to use ts-node, that will actually show errors in ts, and it feels cleaner IMO

Yep indeed, but since it was like that already I've went with consistency, I'll change it in a next PR

scripts/ci/codegen/pushGeneratedCode.ts

millotp

Much cleaner 👍

shortcuts self-assigned this Mar 9, 2022

shortcuts force-pushed the feat/ci-generated-code branch 6 times, most recently from 99adf06 to af4df2c Compare March 10, 2022 15:26

shortcuts requested review from damcou and eunjae-lee March 10, 2022 15:47

shortcuts marked this pull request as ready for review March 10, 2022 15:47

eunjae-lee reviewed Mar 10, 2022

View reviewed changes

shortcuts added 2 commits March 10, 2022 17:42

feat(ci): make ci push generated code

8f3272a

apply changes from suggestion

a8d9aa6

shortcuts force-pushed the feat/ci-generated-code branch from b3c61b7 to a8d9aa6 Compare March 10, 2022 16:51

bump caches?

01733bc

shortcuts requested a review from eunjae-lee March 10, 2022 17:04

eunjae-lee previously approved these changes Mar 11, 2022

View reviewed changes

replace status with diff

2344eb8

shortcuts dismissed eunjae-lee’s stale review via 2344eb8 March 14, 2022 09:09

shortcuts requested review from eunjae-lee and millotp March 14, 2022 09:17

millotp reviewed Mar 14, 2022

View reviewed changes

shortcuts mentioned this pull request Mar 14, 2022

chore: make cache keys cleaner #242

Merged

shortcuts and others added 2 commits March 14, 2022 11:44

inline headRef parameter

b1d5dea

Merge branch 'main' into feat/ci-generated-code

79d650f

millotp approved these changes Mar 14, 2022

View reviewed changes

shortcuts merged commit c174a8b into main Mar 14, 2022

shortcuts deleted the feat/ci-generated-code branch March 14, 2022 10:58

shortcuts added a commit that referenced this pull request Apr 22, 2022

feat(ci): make ci push generated code (#244)

e3e9eb8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ci): make ci push generated code #244

feat(ci): make ci push generated code #244

shortcuts commented Mar 9, 2022 •

edited

Loading

netlify bot commented Mar 9, 2022 •

edited

Loading

eunjae-lee left a comment

millotp left a comment

millotp Mar 14, 2022

shortcuts Mar 14, 2022

millotp Mar 14, 2022

shortcuts Mar 14, 2022 •

edited

Loading

millotp Mar 14, 2022

shortcuts Mar 14, 2022

millotp Mar 14, 2022

shortcuts Mar 14, 2022

millotp Mar 14, 2022

shortcuts Mar 14, 2022

millotp Mar 14, 2022

shortcuts Mar 14, 2022

millotp left a comment

feat(ci): make ci push generated code #244

feat(ci): make ci push generated code #244

Conversation

shortcuts commented Mar 9, 2022 • edited Loading

🧭 What and Why

Changes included:

Stacked PRs:

main:

Other repositories

Next steps

🧪 Test

netlify bot commented Mar 9, 2022 • edited Loading

eunjae-lee left a comment

Choose a reason for hiding this comment

millotp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shortcuts Mar 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

millotp left a comment

Choose a reason for hiding this comment

shortcuts commented Mar 9, 2022 •

edited

Loading

`main`:

netlify bot commented Mar 9, 2022 •

edited

Loading

shortcuts Mar 14, 2022 •

edited

Loading