Skip to content
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.

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

Feat required config #48

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ycjcl868
Copy link

@ycjcl868 ycjcl868 commented Aug 8, 2018

No description provided.

@codecov
Copy link

codecov bot commented Aug 8, 2018

Codecov Report

Merging #48 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #48   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         162    164    +2     
  Branches       56     57    +1     
=====================================
+ Hits          162    164    +2
Impacted Files Coverage Δ
index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 157712a...b8d4fa6. Read the comment docs.

@ycjcl868
Copy link
Author

ycjcl868 commented Aug 8, 2018

The condition is when having a create and edit common interface , I may code like this:

const schema = {
      userId: {
        type: 'int'
      },
      title: {
        required: false,
        type: 'string',
      },
      brief: {
        required: false,
        type: 'string',
      },
      content: {
        required: false,
        type: 'string',
      },
      xxx1: {
          required: false,
           type: 'string',
      },
      ...
    }
ctx.validate(schema, myParams);

index.js Outdated
@@ -33,6 +33,9 @@ class Parameter {
if (opts.validateRoot) {
this.validateRoot = true;
}
this.defaultRequired = opts.defaultRequired !== undefined ?
opts.defaultRequired :
Copy link
Member

Choose a reason for hiding this comment

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

this.defaultRequired = opts.defaultRequired !== false;

Copy link
Author

@ycjcl868 ycjcl868 Aug 8, 2018

Choose a reason for hiding this comment

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

thx for cr ! ~

index.js Outdated
if (rule.required !== false) {
var required = rule.required !== undefined ?
rule.required :
this.defaultRequired;
Copy link
Member

Choose a reason for hiding this comment

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

放一行吧,这样缩进有点奇怪

Copy link
Author

@ycjcl868 ycjcl868 Aug 8, 2018

Choose a reason for hiding this comment

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

嗯嗯,已修改。 839d425

docs: options.defaultRequired
@fengmk2
Copy link
Member

fengmk2 commented Sep 19, 2018

rebase ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants