Conversation
skladany
left a comment
skladany
left a comment
There was a problem hiding this comment.
Nits, typos, and improvements, but nothing blocking 🏃♂️
| boolean: {type: 'Boolean' as const}, | ||
| } | ||
| private static sanitizePath(pathStr: string): string { | ||
| const trimmed = pathStr.split('/') |
There was a problem hiding this comment.
Might want to add a .map(segment => segment.trim()) here to avoid accidental white space end up in the sanitized path
There was a problem hiding this comment.
Also, I know the CLI specifically says to enter a base-path ...but might it be worth checking for schema/host and stripping that out too (or generating an error at the CLI section)?
There was a problem hiding this comment.
This one is just for the path not for the full url. Trim is a good call
src/tools.ts
Outdated
| schemaV4: string; | ||
| } | ||
|
|
||
| export interface GenerageSchemaFromOpenApiSpecOptions { |
There was a problem hiding this comment.
typo: GenerageSchemaFromOpenApiSpecOptions -> GenerateSchemaFromOpenApiSpecOptions
| const apiSpecFile = argv['api-spec'] as string; | ||
| const namespace = argv.namespace as string; | ||
| const mappingType = argv['mapping-type'] as MappingType; | ||
| const serverBasePath = typeof argv['base-path'] === 'string' ? argv['base-path'] : undefined; |
There was a problem hiding this comment.
nit: might this make more sense as just basePath to be consistent with CLI option base-path? (or change the option to server-base-path)
src/tools.ts
Outdated
| @@ -32,14 +44,42 @@ export class Tools { | |||
| * @param namespace cedar namespace for your application | |||
There was a problem hiding this comment.
These (and other @param comments elsewhere) are no longer accurate since you're using an options object now
Swolebrain
force-pushed
the
support-basepath
branch
from
b98bec8 to
4334a30
Compare
2 participants
Enhance schema generator and cli tool to support basepaths in openapi v3 specs.