chore(cts): update dependencies on cts generation for javascript #490

eunjae-lee · 2022-05-10T13:41:33Z

🧭 What and Why

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

Changes included:

cts generate command now always updates the dependencies in tests/output/javascript/package.json.

I will probably send a separate PR for java and php. I'd like to get this javascript validated first before moving onto other languages.

netlify · 2022-05-10T13:41:40Z

✅ Deploy Preview for api-clients-automation canceled.

Name	Link
🔨 Latest commit	`70fdd86`
🔍 Latest deploy log	https://app.netlify.com/sites/api-clients-automation/deploys/6282211162e76c000954484a

algolia-bot · 2022-05-10T13:42:37Z

✗ The generated branch has been deleted.

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

tests/CTS/methods/requests/templates/javascript/package.mustache

generators/src/main/java/com/algolia/codegen/cts/AlgoliaCtsGenerator.java

millotp

Looks good !

millotp · 2022-05-11T14:49:49Z

tests/CTS/methods/requests/templates/javascript/package.mustache

+  },
+  "dependencies": {
+    {{#packageVersionMap}}
+    "@experimental-api-clients-automation/algoliasearch-lite": "{{@experimental-api-clients-automation/algoliasearch-lite}}",


you could iterate on a array of object, and have "{{packageName}}": "{{packageVersion}}

I thought about it, but just left it like that, to be explicit about which dependencies we use for the package. WDTY?

we can be as explicit but in the generator :)

millotp · 2022-05-11T14:51:54Z

generators/src/main/java/com/algolia/codegen/cts/AlgoliaCtsGenerator.java

@@ -84,6 +84,12 @@ public void processOpts() {
          clientName + testConfig.extension
        )
      );
+
+      if (language.equals("javascript")) {


Can you make this generic with a class that will add the correct supporting files and and data ?

Can you give me an example of what the class would look like?

interface VersionManager { void addSupportingFiles(...); void addData(...); } class JavascriptVersionManager implements VersionManager { //your code }

And instanciate the correct one in AlgoliaCtsGenerator.java based on the language.

👍 coming soon

e532455

@millotp what do you think?

millotp · 2022-05-11T14:52:29Z

generators/src/main/java/com/algolia/codegen/Utils.java

@@ -149,4 +153,15 @@ public static void generateServer(
      System.exit(1);
    }
  }
+
+  public static JsonNode readJsonFile(String filePath) {


Very cool ! can you replace other instance of the code where we load a json by this method ?

generators/src/main/java/com/algolia/codegen/cts/CtsManagerFactory.java

shortcuts · 2022-05-12T13:23:46Z

tests/output/javascript/package.json

@@ -5,17 +5,17 @@
    "test": "jest"


We should maybe sort those deps otherwise it will be sorted locally and then unsorted when bumped etc

ah or maybe just the yarn install that will come after will sort them actually

Actually I was going to ask this question, but why aren't this deps sorted automatically? I thought formatting after the generation would do it, but didn't.

I believe it's yarn that sorts the dependencies in the package.json

I just found it:

After the cts generation, it runs this formatter: https://github.com/algolia/api-clients-automation/blob/7c434506d7d6f23f044841024b0dbc8e9af26514/scripts/formatter.ts#L16:L16

and it runs:

yarn eslint --ext=ts tests/output/javascript/src --fix --no-error-on-unmatched-pattern

that's why tests/output/javascript/package.json is being excluded from the eslint.

ahhh yes we can maybe add an extra line for the package.json if it works then

d3c14dc

What do you think?

generators/src/main/java/com/algolia/codegen/cts/CtsManagerFactory.java

millotp

Beautiful ! The version logic has changed a little bit on main but it should make it easier here, you can add utils the same way as in js

millotp · 2022-05-12T13:27:36Z

generators/src/main/java/com/algolia/codegen/cts/AlgoliaCtsGenerator.java

@@ -63,31 +67,34 @@ public void processOpts() {
    language = (String) additionalProperties.get("language");
    client = (String) additionalProperties.get("client");
    packageName = (String) additionalProperties.get("packageName");
+    ctsManager = CtsManagerFactory.getManager(language);


generators/src/main/java/com/algolia/codegen/cts/CtsManager.java

generators/src/main/java/com/algolia/codegen/cts/CtsManagerFactory.java

eunjae-lee · 2022-05-12T14:45:50Z

I'll mark it as ready for review once I make the CI pass.

millotp

A last few comments 😅 also can you add the package.json of the test to generation.config.json ?

millotp · 2022-05-12T15:57:46Z

generators/src/main/java/com/algolia/codegen/cts/AlgoliaCtsGenerator.java

+      bundle.put("packageDependencies", ctsManager.getPackageDependencies());
+      ctsManager.addExtraToBundle(bundle);


it would be clearer to combine those into one like addSupportingData(bundle), and let the manager add what it wants

it makes sense 0303223

millotp · 2022-05-12T16:22:26Z

generators/src/main/java/com/algolia/codegen/cts/manager/CtsManager.java

+      if (!additionalProperties.has("packageVersion")) {
+        continue;
+      }
+      String packageName = additionalProperties.get("packageName").asText();


packageName is a lie, it's not used for anything other than js because it doesn't change, unless we know other languages that will be split in clients, I think we can remove this notion from the CTS, and remove them from clients.config.json (but keep them for js)

And have this code that his specific to JS in the JavascriptCtsManager

Bonne idée 2d04ade

shortcuts

Looks clean!! Pierre said most of it, just one comment more :D

shortcuts · 2022-05-16T07:17:27Z

generators/src/main/java/com/algolia/codegen/cts/manager/JavaScriptCtsManager.java

+          "@experimental-api-clients-automation/algoliasearch-lite",
+          "@experimental-api-clients-automation/client-abtesting",
+          "@experimental-api-clients-automation/client-analytics",
+          "@experimental-api-clients-automation/client-common",
+          "@experimental-api-clients-automation/client-insights",
+          "@experimental-api-clients-automation/client-personalization",
+          "@experimental-api-clients-automation/client-predict",
+          "@experimental-api-clients-automation/client-query-suggestions",
+          "@experimental-api-clients-automation/client-search",
+          "@experimental-api-clients-automation/client-sources",
+          "@experimental-api-clients-automation/recommend",
+          "@experimental-api-clients-automation/requester-node-http"


Maybe this list can be guessed from the packageName in openapitools.json?

done here 2d04ade

millotp · 2022-05-16T09:37:44Z

also can you add the package.json of the test to generation.config.json ?

pls

eunjae-lee · 2022-05-16T10:02:08Z

also can you add the package.json of the test to generation.config.json ?

pls

Oh I missed that part. 70fdd86

chore(cts): accept an option to update versions

c7a8575

eunjae-lee commented May 10, 2022

View reviewed changes

tests/CTS/methods/requests/templates/javascript/package.mustache Outdated Show resolved Hide resolved

millotp reviewed May 10, 2022

View reviewed changes

generators/src/main/java/com/algolia/codegen/cts/AlgoliaCtsGenerator.java Outdated Show resolved Hide resolved

eunjae-lee mentioned this pull request May 10, 2022

chore(ci): update cts and playground dependencies on release #467

Closed

eunjae-lee added 3 commits May 10, 2022 18:10

chore: WIP

ea23543

chore: extract to Utils

8e15f43

chore: if condition for js

1d88899

eunjae-lee changed the title ~~chore(cts): accept an option to update versions~~ chore(cts): update dependencies on cts generation for javascript May 11, 2022

eunjae-lee marked this pull request as ready for review May 11, 2022 14:45

millotp reviewed May 11, 2022

View reviewed changes

eunjae-lee added 2 commits May 11, 2022 17:04

chore: clean up duplicated code

de0c770

chore: introduce CtsManager

e532455

eunjae-lee commented May 12, 2022

View reviewed changes

generators/src/main/java/com/algolia/codegen/cts/CtsManagerFactory.java Outdated Show resolved Hide resolved

shortcuts reviewed May 12, 2022

View reviewed changes

chore: run formatter on java generators

b136ec6

millotp reviewed May 12, 2022

View reviewed changes

eunjae-lee added 6 commits May 12, 2022 16:05

chore: split classes into multiple files

e271b22

chore: remove unused code and run formatter

7c43450

chore: update formatter to include json

d3c14dc

chore: remove redundant check

7a8512b

Merge branch 'main' into chore/update-cts

5511ea3

chore: adjust to the latest main branch

62fc1bb

eunjae-lee marked this pull request as draft May 12, 2022 14:45

eunjae-lee marked this pull request as ready for review May 12, 2022 14:57

eunjae-lee requested review from millotp and shortcuts May 12, 2022 14:58

millotp reviewed May 12, 2022

View reviewed changes

shortcuts reviewed May 16, 2022

View reviewed changes

eunjae-lee added 2 commits May 16, 2022 10:55

chore: wrap with addDataToBundle

0303223

move the implementation of getPackageDependencies to the js level

2d04ade

chore: update generation list

70fdd86

millotp approved these changes May 16, 2022

View reviewed changes

eunjae-lee merged commit 27f5e4a into main May 16, 2022

eunjae-lee deleted the chore/update-cts branch May 16, 2022 12:01

eunjae-lee mentioned this pull request May 16, 2022

chore(cts): update dependency on cts generation for java #505

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(cts): update dependencies on cts generation for javascript #490

chore(cts): update dependencies on cts generation for javascript #490

eunjae-lee commented May 10, 2022 •

edited

Loading

netlify bot commented May 10, 2022 •

edited

Loading

algolia-bot commented May 10, 2022 •

edited

Loading

millotp left a comment

millotp May 11, 2022

eunjae-lee May 11, 2022

millotp May 11, 2022

millotp May 11, 2022

eunjae-lee May 11, 2022

millotp May 11, 2022

eunjae-lee May 11, 2022

eunjae-lee May 12, 2022

millotp May 11, 2022

eunjae-lee May 12, 2022

shortcuts May 12, 2022

shortcuts May 12, 2022

eunjae-lee May 12, 2022

shortcuts May 12, 2022

eunjae-lee May 12, 2022

shortcuts May 12, 2022

eunjae-lee May 12, 2022

millotp left a comment

millotp May 12, 2022

eunjae-lee commented May 12, 2022

millotp left a comment •

edited

Loading

millotp May 12, 2022

eunjae-lee May 16, 2022

millotp May 12, 2022

millotp May 12, 2022

eunjae-lee May 16, 2022

shortcuts left a comment

shortcuts May 16, 2022

eunjae-lee May 16, 2022

millotp commented May 16, 2022 •

edited

Loading

eunjae-lee commented May 16, 2022

		bundle.put("packageDependencies", ctsManager.getPackageDependencies());
		ctsManager.addExtraToBundle(bundle);

chore(cts): update dependencies on cts generation for javascript #490

chore(cts): update dependencies on cts generation for javascript #490

Conversation

eunjae-lee commented May 10, 2022 • edited Loading

🧭 What and Why

Changes included:

netlify bot commented May 10, 2022 • edited Loading

✅ Deploy Preview for api-clients-automation canceled.

algolia-bot commented May 10, 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

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

Choose a reason for hiding this comment

eunjae-lee commented May 12, 2022

millotp left a comment • 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

shortcuts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

millotp commented May 16, 2022 • edited Loading

eunjae-lee commented May 16, 2022

eunjae-lee commented May 10, 2022 •

edited

Loading

netlify bot commented May 10, 2022 •

edited

Loading

algolia-bot commented May 10, 2022 •

edited

Loading

millotp left a comment •

edited

Loading

millotp commented May 16, 2022 •

edited

Loading