Skip to content

chore(repository-json-schema): replace JsonDefinition with JSONSchema6 #1286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
shimks merged 1 commit into from
Apr 26, 2018
Merged

chore(repository-json-schema): replace JsonDefinition with JSONSchema6 #1286

shimks merged 1 commit into from
Apr 26, 2018

Conversation

@shimks
Copy link
Contributor

@shimks shimks commented Apr 26, 2018 •
edited
Loading

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

virkt25
virkt25 reviewed Apr 26, 2018
View reviewed changes
packages/openapi-v3/src/json-to-schema.ts Outdated
// License text available at https://opensource.org/licenses/MIT

import {JsonDefinition} from '@loopback/repository-json-schema';
import {JSONSchema6} from '@loopback/repository-json-schema';
Copy link
Contributor

@virkt25 virkt25 Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import {JSONSchema6 as JSONSchema}

@shimks shimks force-pushed the replace-json-schema branch from dbac19f to 21289f5 Compare April 26, 2018 18:57
b-admike
b-admike reviewed Apr 26, 2018
View reviewed changes
packages/repository-json-schema/src/index.ts Outdated
export * from './build-schema';

import {JSONSchema6} from 'json-schema';
export {JSONSchema6};
Copy link
Contributor

@b-admike b-admike Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSONSchema as we talked about here?

@raymondfeng
Copy link
Contributor

raymondfeng commented Apr 26, 2018

lodash.includes dependency is not used. Please remove it.

b-admike
b-admike approved these changes Apr 26, 2018
View reviewed changes
packages/openapi-v3/src/json-to-schema.ts

return result;
}

Copy link
Contributor

@b-admike b-admike Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TSDocs here perhaps?

Copy link
Contributor Author

@shimks shimks Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@shimks
Copy link
Contributor Author

shimks commented Apr 26, 2018 •
edited
Loading

@raymondfeng It is used in https://github.com/strongloop/loopback-next/pull/1286/files#diff-432476dd2d984e441d8b4e0af39a39beR68. We could just use includes on Array.prototype if you want to cut down the packages we use.

@shimks shimks force-pushed the replace-json-schema branch from 21289f5 to 6b04f99 Compare April 26, 2018 19:15
b-admike
b-admike reviewed Apr 26, 2018
View reviewed changes
packages/openapi-v3/src/json-to-schema.ts
* Helper function used to interpret boolean values as JSON Schemas.
* See http://json-schema.org/draft-06/json-schema-release-notes.html
* @param jsonOrBool converts boolean values into their representative JSON Schemas
*/
Copy link
Contributor

@b-admike b-admike Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! nitpick: missing @returns

@shimks shimks force-pushed the replace-json-schema branch from 6b04f99 to e8a1e51 Compare April 26, 2018 19:25
@raymondfeng
Copy link
Contributor

raymondfeng commented Apr 26, 2018

@raymondfeng It is used in https://github.com/strongloop/loopback-next/pull/1286/files#diff-432476dd2d984e441d8b4e0af39a39beR68. We could just use includes on Array.prototype if you want to cut down the packages we use.

We should use Array.prototype.includes

@shimks shimks force-pushed the replace-json-schema branch from e8a1e51 to 34607ea Compare April 26, 2018 20:24
@shimks shimks force-pushed the replace-json-schema branch from 34607ea to c8fc00e Compare April 26, 2018 20:26
@shimks shimks merged commit e9e6b12 into master Apr 26, 2018
@shimks shimks deleted the replace-json-schema branch April 26, 2018 20:58
bajtos
bajtos reviewed Apr 27, 2018
View reviewed changes
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@bajtos bajtos mentioned this pull request Apr 27, 2018
Merged
7 tasks
@raymondfeng raymondfeng mentioned this pull request Apr 27, 2018
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raymondfeng raymondfeng raymondfeng approved these changes

+3 more reviewers

@bajtos bajtos bajtos left review comments

@virkt25 virkt25 virkt25 approved these changes

@b-admike b-admike b-admike approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace type definition being imported by repository-json-schema

6 participants

@shimks @raymondfeng @bajtos @virkt25 @b-admike