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: support inner not, not in simple types #61

Closed
wants to merge 19 commits into from

Conversation

vankop
Copy link
Member

@vankop vankop commented Sep 28, 2019

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes #65

#42
part of 1st proposal.

Breaking Changes

no

Additional Info

Nothing

@codecov
Copy link

codecov bot commented Sep 28, 2019

Codecov Report

Merging #61 into master will decrease coverage by 0.56%.
The diff coverage is 94.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
- Coverage   98.12%   97.56%   -0.57%     
==========================================
  Files           5        6       +1     
  Lines         639      697      +58     
  Branches      267      298      +31     
==========================================
+ Hits          627      680      +53     
- Misses         12       15       +3     
- Partials        0        2       +2     
Impacted Files Coverage Δ
src/validate.js 100.00% <ø> (ø)
src/ValidationError.js 96.67% <89.06%> (-0.99%) ⬇️
src/util/hints.js 100.00% <100.00%> (ø)

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 d8d7cbd...b75bdcd. Read the comment docs.

src/util/Range.js Outdated Show resolved Hide resolved
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Good job, let's avoid here logic for not and do this on other PR?

@vankop vankop changed the title feat: support not for numbers, "smart" inner not, "smart" number range feat: support inner not Sep 28, 2019
@vankop vankop changed the title feat: support inner not feat: support inner not, not in numbers Sep 28, 2019
@vankop
Copy link
Member Author

vankop commented Sep 28, 2019

what is problem with CI? coverage decrease?

@alexander-akait
Copy link
Member

alexander-akait commented Sep 30, 2019

Yep, don't know why, you can ignore this

@vankop
Copy link
Member Author

vankop commented Oct 3, 2019

@evilebottnawi I suggest to split not functionality in several PRs since logic for arrays and objects need a lot of tests. I will add tests too this PR also.

@vankop vankop changed the title feat: support inner not, not in numbers feat: support inner not, not in simple types Oct 4, 2019
@vankop
Copy link
Member Author

vankop commented Oct 4, 2019

@evilebottnawi I think this is ready to review 😁

@alexander-akait
Copy link
Member

/cc @vankop sorry for delay, can we fix it #65 in a separate PR for chanelog and clean git history? Anyway thanks for good job!

@vankop
Copy link
Member Author

vankop commented Oct 9, 2019

@evilebottnawi I can do it, but later. Busy right now.

@alexander-akait
Copy link
Member

@vankop great

# Conflicts:
#	src/ValidationError.js
#	test/__snapshots__/index.test.js.snap
@alexander-akait
Copy link
Member

@vankop let's wait a bit with this, at the moment I'm adding JSDoc for all project and continue after this 👍

@vankop
Copy link
Member Author

vankop commented Oct 17, 2019

TypeScript 🤩🤗

@alexander-akait
Copy link
Member

/cc @vankop we can continue development too 😄

vankop added 3 commits April 16, 2020 23:24
# Conflicts:
#	src/ValidationError.js
#	test/__snapshots__/index.test.js.snap
#	test/index.test.js
# Conflicts:
#	src/ValidationError.js
#	test/__snapshots__/index.test.js.snap
npm audit fix

npm audit fix one more
@vankop
Copy link
Member Author

vankop commented Apr 16, 2020

@evilebottnawi so much work is done =) need rereview

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Oh, a lot of changes, very hard to review, maybe we can separate this PR on parts? Also it will be good for git history?

@alexander-akait
Copy link
Member

alexander-akait commented Apr 17, 2020

@vankop friendly ping

# Conflicts:
#	package-lock.json
@vankop
Copy link
Member Author

vankop commented Apr 17, 2020

I will try to reduce

@alexander-akait
Copy link
Member

Will be great, anyway thanks for a job, if you can't reduce ping me and I will try to review this big PR

This was referenced May 15, 2020
@vankop vankop closed this May 18, 2020
@vankop vankop deleted the smart-not-case branch May 18, 2020 12:54
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.

minLength maxLength wrong format string
3 participants