chore: codegen improvements #820
Conversation
This makes it easier to understand what has actually been updated in the snapshot.
To avoid yarn complaining.
| @@ -1,14 +1,14 @@ | |||
| { | |||
| "private": true, | |||
| "name": "@cognite/sdk-codegen", | |||
| "version": "0.0.0-development", | |||
There was a problem hiding this comment.
why did you add version?
There was a problem hiding this comment.
Rather: can you verify that this won't publish/release the codegen package on changes?
There was a problem hiding this comment.
The commit message should explain. Yarn complains when packages does not contain a version (it's required for a package.json file)
There was a problem hiding this comment.
Version is only required if you plan to publish a package. I don't believe that's a goal here.
There was a problem hiding this comment.
Yarn currently complains:
$ yarn
warning Missing version in workspace at "/home/henrste/projects/cognitedata/cognite-sdk-js/packages/codegen", ignoring.
I don't want to see this warning every time I run yarn to install deps. It clutters other potential important warnings, and is just annoying.
There was a problem hiding this comment.
Seems like a bug in yarn then, would make more sense to do a PR to yarn instead of adjusting our codebase. I really don't think we should do releases for this package, which I'd argue is more significant than a unnecessary warning.
There was a problem hiding this comment.
"private": true will prevent Lerna/NPM from publishing the package. See https://github.com/lerna/lerna/tree/main/commands/publish#usage and https://docs.npmjs.com/cli/v8/configuring-npm/package-json#private
We have --no-private in the lerna version command (see release.yml workflow), so this package should not be changed by that neither.
There was a problem hiding this comment.
Edit:
looking at 1704e1d
It seems there's no issue. So go ahead and keep version in 👍 . I still think the better approach would be to send a patch to yarn as that's the problem, as having a version for a package that is never published doesn't make sense imho.
There was a problem hiding this comment.
I've tested this against a local registry. The version and publish command does not touch the codegen package at all, so this should be fine. I can't comment on that issue, but this might be because we are using more flags and specific variants of the commands.
Successfully published:
- @cognite/sdk-beta@5.1.2
- @cognite/sdk-core@4.4.1
- @cognite/sdk-playground@5.3.2
- @cognite/sdk@7.6.2
lerna success published 4 packages
|
Anything you find outstanding for this now @andersfylling? |
|
Looks good! Let's get this merged and I'll ping tooling to review it. Hopefully we can this code gen out and see if any problems arise. |
* docs: add documentation * chore: add dependencies * chore: add codegen package * chore: configure ts-node * chore: yarn commands/scripts using yargs * chore: add codegen logic * chore: remove build command * chore: add nodejs version guard * test: use node 14, 16 and 18 * test: drop v18 * test: cyclic references * chore: setup documents service * chore: minor fixes * docs: cleanup * test: github action to verify generated files * chore: generated shared types for package * chore: cleanup after review comments * chore: update packages/codegen/src/codegen.ts Co-authored-by: Henrik Steen <henrist@henrist.net> * test: fix expected output ts file * test: fix expected output ts file * test: update .github/workflows/code-gen.yaml Co-authored-by: Henrik Steen <henrist@henrist.net> * chore: minor improvements * chore: minor improvements * style: fix * chore: run code gen + fixes * chore: wip * chore: simpler commands + always read from config * chore: enable codegen for playground & playground/documents service * fix: capitalize first char of schemas * chore: sort json spec * chore: fix potential null issue + regenerate types * test: fix sorted values * test: add --detectOpenHandles * chore: remove dead args * chore: ignore exports.gen.ts * chore: remove openHandle * chore: simply typeof undefined check * chore: use null check instead of undefined * chore: title case error messages * chore: comments * chore: cleanup variable names * chore: rename SnapshotOnline * chore: general cleanup + use interface instead of type * chore: remove dead subcommands * chore: rename code-gen.yaml to codegen.yaml * chore: rename code-gen.yaml to codegen.yaml * chore: move snapshot command into snapshot.ts * chore: move config command into configuration.ts * chore: rename commands.ts to generate.ts * chore: rename commands.ts to generate.ts * chore: cleanup + remove need to cast config types * chore: update snapshots and rerun code generation * chore: add eslintignore for playground pkg * chore: codegen improvements (#820) * chore: rework and simplify CLI for codegen * chore: drop the command classes in favor of top level functions * chore: rewrite docs * chore: remove obsolete esCheck run-script * chore: remove obsolete tsconfig.build.json file * chore: rename snapshot file and set linguist-generated * chore: pretty format openapi snapshot This makes it easier to understand what has actually been updated in the snapshot. * chore: minor typo * chore: rephrase "open api" * chore: remove node version check but add engines * chore: set version in codegen package To avoid yarn complaining. * chore: re-hide the snapshot file * chore: fix configure description * style: fix * chore: fix workflow file * test: don't use node v12 on the codegen package * chore: move engines directive to codegen * chore: update * chore: update readme Co-authored-by: Henrik Steen <henrist@henrist.net> Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Ornellas <93260466+cognite-ornellas@users.noreply.github.com>
See the individual commits