feat(script): variadic parameters #472

shortcuts · 2022-05-05T10:49:10Z

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-238

Allow variadic parameters in the CLI, to reduce the number of jobs/complexity of the CI.

Changes included:

(Sorry for the huge PR, it was hard not to break things here and there)

The goal of this PR is to remove any logic of the client path at the CI level, which what makes it extremely verbose.

New CLI usage:

before: yarn docker generate javascript search && yarn docker generate javascript recommend
now: yarn docker generate javascript search recommend

This change allow us to scope every clients at their base folder level.

Side effects:

Kind of use Language type in the CLI https://algolia.atlassian.net/browse/APIC-385
Remove double run of eslint for JavaScript client (hyphen issue in jsdoc)
Remove common spec bundling before each jobs
Remove JS utils bundling before each jobs
Single gen matrix (size of Language)
Matrix on CTS (size of Language)

🧪 Test

CI :D

netlify · 2022-05-05T10:49:14Z

✅ Deploy Preview for api-clients-automation canceled.

Name	Link
🔨 Latest commit	`56614ef`
🔍 Latest deploy log	https://app.netlify.com/sites/api-clients-automation/deploys/6273ec14c449f6000842f5b6

algolia-bot · 2022-05-05T10:50:10Z

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.

millotp

Amazing ! One obvious improvement would be to run the CTS directly after a gen is done, we don't have to wait for all languages, but it's already a huge upgrade !

millotp · 2022-05-05T11:39:24Z

.github/actions/restore-artifacts/action.yml

+  This composite restore all of our artifacts, at their right path.
+
+inputs:
+  type:


we go full circle with the cache ahah

.github/actions/restore-artifacts/action.yml

millotp · 2022-05-05T11:46:41Z

.github/workflows/check.yml

        if: ${{ steps.cache.outputs.cache-hit != 'true' }}
-        run: git --no-pager diff
+        run: yarn cli build specs ${{ matrix.client.toRun }}


You should remove the matrix and just use $ {{ fromJSON(needs.setup.outputs.SPECS_MATRIX).client.toRun }}

It's more verbose I'm not a fan of it for readability purposes D:

that way we don't have to click on the CI to see the details !

millotp · 2022-05-05T11:53:11Z

scripts/config.ts

  return clientsConfig[language]?.tests?.extension;
 }

-export function getTestOutputFolder(language: string): string | undefined {
+export function getTestOutputFolder(language: string): string {
  return clientsConfig[language]?.tests?.outputFolder;


how is this correct ? ? will return undefined

The types are really wrong in some places, we need to fix that in a later PR 6920796

millotp · 2022-05-05T11:54:00Z

scripts/index.ts

  if (language === 'all') {
    langsTodo = LANGUAGES;
  }
-  if (client === 'all') {
+  if (client[0] === 'all') {


maybe throw if the first client is all and the length is more than one

scripts/index.ts

millotp · 2022-05-05T12:03:45Z

scripts/ci/githubActions/createMatrix.ts

+  const matrix = LANGUAGES.reduce(
+    (curr, lang) => ({
+      ...curr,
+      [lang]: {
+        path: getLanguageFolder(lang),
+        toRun: [],
+        cacheToCompute: [],
+      },
+    }),
+    {} as Record<Language, ToRunMatrix>
+  );


Suggested change

const matrix = LANGUAGES.reduce(

(curr, lang) => ({

...curr,

[lang]: {

path: getLanguageFolder(lang),

toRun: [],

cacheToCompute: [],

},

}),

{} as Record<Language, ToRunMatrix>

);

const matrix: Record<string, ToRunMatrix> = Object.fromEntries(

LANGUAGES.map((lang) => [

lang,

{

path: getLanguageFolder(lang),

toRun: [],

cacheToCompute: [],

},

])

);

There is no good way to do it and keep the type correct without casting, I don't know which one is more readable

shortcuts · 2022-05-05T12:36:32Z

One obvious improvement would be to run the CTS directly after a gen is done, we don't have to wait for all languages, but it's already a huge upgrade !

Good idea!!

millotp

Looking good ! It's weird that the size of the artifacts is the uncompressed size at upload and download, but when I actually download an artifacts it's the correct size

millotp · 2022-05-05T15:11:42Z

.github/actions/restore-artifacts/action.yml

+
+inputs:
+  type:
+    description: Type of artifacts to restore (`all` | `specs` | `javascript` | `js-utils` | `java` | `php`)


Can you remove 'java' and 'php' if they are not used ?

shortcuts self-assigned this May 5, 2022

shortcuts force-pushed the feat/cli-variadic branch from e21c23b to 88ce6dc Compare May 5, 2022 10:49

shortcuts changed the title ~~feat(cli): variadic parameters~~ feat(script): variadic parameters May 5, 2022

feat(cli): variadic parameters

b45fd07

shortcuts force-pushed the feat/cli-variadic branch from 88ce6dc to b45fd07 Compare May 5, 2022 11:10

shortcuts requested review from a team, eunjae-lee and millotp and removed request for a team May 5, 2022 11:35

shortcuts marked this pull request as ready for review May 5, 2022 11:35

millotp reviewed May 5, 2022

View reviewed changes

no CTS matrix

2005c2c

shortcuts added 3 commits May 5, 2022 14:40

Merge branch 'main' into feat/cli-variadic

8c879d2

better type language

6920796

review

18c79af

shortcuts requested a review from millotp May 5, 2022 13:45

shortcuts enabled auto-merge (squash) May 5, 2022 15:13

millotp reviewed May 5, 2022

View reviewed changes

update description

56614ef

shortcuts requested a review from millotp May 5, 2022 15:24

millotp approved these changes May 5, 2022

View reviewed changes

shortcuts merged commit 30111c9 into main May 5, 2022

shortcuts deleted the feat/cli-variadic branch May 5, 2022 15:34

This was referenced May 19, 2022

chore: release 2022-05-19 #518

Closed

chore: release 2022-05-19 #520

Closed

chore: release 2022-05-23 #533

Closed

chore: prepare release 2022-05-24 #541

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(script): variadic parameters #472

feat(script): variadic parameters #472

shortcuts commented May 5, 2022

netlify bot commented May 5, 2022 •

edited

Loading

algolia-bot commented May 5, 2022 •

edited

Loading

millotp left a comment

millotp May 5, 2022

millotp May 5, 2022

shortcuts May 5, 2022

millotp May 5, 2022

millotp May 5, 2022

shortcuts May 5, 2022

millotp May 5, 2022

millotp May 5, 2022

shortcuts commented May 5, 2022

millotp left a comment

millotp May 5, 2022

shortcuts May 5, 2022

feat(script): variadic parameters #472

feat(script): variadic parameters #472

Conversation

shortcuts commented May 5, 2022

🧭 What and Why

Changes included:

🧪 Test

netlify bot commented May 5, 2022 • edited Loading

✅ Deploy Preview for api-clients-automation canceled.

algolia-bot commented May 5, 2022 • edited Loading

✗ The generated branch has been deleted.

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

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

shortcuts commented May 5, 2022

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

netlify bot commented May 5, 2022 •

edited

Loading

algolia-bot commented May 5, 2022 •

edited

Loading