Add readme tag support #115
Conversation
| if (Info != null && | ||
| previousDefinition.Info != null && | ||
| Info.Version != null && | ||
| previousDefinition.Info.Version != null) |
There was a problem hiding this comment.
you can use sequential check to simpify :
Info?.Version != null && previousDefinition.Info?.Version != null #Closed
lib/commands/oad.js
Outdated
| default: true | ||
| }, | ||
| o: { | ||
| alias: 'old-tag-name', |
There was a problem hiding this comment.
old-tag-name [](start = 12, length = 12)
oad uses camelCase parameters
| n: { | ||
| alias: 'new-tag-name', | ||
| describe: 'The tag name for the readme. If include also indicates that the old spec file is a readme file', | ||
| } |
There was a problem hiding this comment.
also we need to update the readme for the new option and the changelog
There was a problem hiding this comment.
| * @param {string} swaggerPath Path to the specification file. | ||
| * | ||
| * @param {string} outputFileName Name of the output file to which autorest outputs swagger-doc. | ||
| * |
lib/validators/openApiDiff.js
Outdated
| let autoRestCmd = `${self.autoRestPath()} --input-file=${swaggerPath} --output-artifact=swagger-document.json --output-file=${outputFileName} --output-folder=${outputFolder}`; | ||
| var autoRestCmd; | ||
| if(tagName) { | ||
| autoRestCmd = `${self.autoRestPath()} ${swaggerPath} --tag=${tagName} --output-artifact=swagger-document.json --output-file=${outputFileName} --output-folder=${outputFolder}`; |
There was a problem hiding this comment.
nit: ternary operator instead tslint
cli.js
Outdated
| alias: 'newTagName', | ||
| describe: `The tag name for the new specification file. If include it ` + | ||
| `indicates that the new spec file is a readme file` | ||
| }) |
There was a problem hiding this comment.
here should be only global parameters ( i.e. not tied to a command), those should go in the specific command folder. You can check https://github.com/yargs/yargs/blob/master/docs/advanced.md for documentation on how to structure yargs.
lib/validators/openApiDiff.js
Outdated
| var promise1 = self.processViaAutoRest(oldSwagger, 'old'); | ||
| var promise2 = self.processViaAutoRest(newSwagger, 'new'); | ||
| var promise1 = self.processViaAutoRest(oldSwagger, 'old', self.options.oldTagName); | ||
| var promise2 = self.processViaAutoRest(newSwagger, 'new', self.options.newTagName); |
There was a problem hiding this comment.
self.options.newTagName); [](start = 62, length = 25)
it doesn't really make sense to have the tag name in the constructor right ? that would mean if we call compare multiple times there will be static tags. Maybe add them as optional parameters to compare
There was a problem hiding this comment.
This makes sense, we send in two swaggers and a tag is associated with a swagger. This seems like an ok implementation.
lib/commands/oad.js
Outdated
|
|
||
| let compareFunc; | ||
| if(oldTag && newTag) { | ||
| compareFunc = validate.compareTags(oldSpec, oldTag, oldSpec, newTag, vOptions); |
| log.filepath = options.logFilepath || log.filepath; | ||
| let openApiDiff = new OpenApiDiff(options); | ||
|
|
||
| return openApiDiff.compare(oldSwagger, newSwagger, oldTag, newTag); |
There was a problem hiding this comment.
since this will always be a Readme, maybe we can name it something like oldReadme newReadme
There was a problem hiding this comment.
Lets look at this in a subsequent checkin.
2 participants
No description provided.