Skip to content
This repository was archived by the owner on Dec 10, 2021. It is now read-only.

Add file ref support #7

Merged
merged 11 commits into from
May 7, 2019
Merged

Add file ref support #7

merged 11 commits into from
May 7, 2019

Conversation

Deadarius
Copy link

@Deadarius Deadarius commented May 7, 2019

  • Refactor: replace supplied utils with lodash
  • Type improvements
  • Use json-schema-ref-parser to combine bundle schemas while preserving $refs
  • Now schema can be supplied as a path to file

Lint errors to be fixed

@Deadarius Deadarius force-pushed the add-file-ref-support branch from e1095f2 to 9544c11 Compare May 7, 2019 04:59
Copy link

@lwcooper lwcooper left a comment

Choose a reason for hiding this comment

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

Lack of tests is a bit worrying in this whole module really! But not gonna block this PR being merged

@@ -1,10 +1,11 @@
import { JSONSchema7 } from 'json-schema';
import _ = require('lodash');
Copy link

Choose a reason for hiding this comment

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

this syntax looks weird. how does it work? I've never seen it before

also, we normally use ramda 🙈 But I can see how much it makes sense to just replace the existing utils with lodash here 👍

Copy link
Author

Choose a reason for hiding this comment

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

This is an open source project, I would prefer to keep it easy for people to contribute - lodash is more aligned with this idea than ramda


/**
* Constructor
* @param serverless
* @param options
*/
constructor (serverless, options) {
constructor (serverless: IFullServerless, options) {
Copy link

Choose a reason for hiding this comment

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

how come options doesn't need a type?

Copy link
Author

Choose a reason for hiding this comment

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

This project is not configured(yet?) to use strict typescript transpilation.

@Deadarius
Copy link
Author

RE: test covereage - #13

@Deadarius Deadarius merged commit 2f671fb into master May 7, 2019
@Deadarius Deadarius deleted the add-file-ref-support branch May 7, 2019 23:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants