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

fixes examples folder #47

Merged
merged 2 commits into from Mar 25, 2018
Merged

fixes examples folder #47

merged 2 commits into from Mar 25, 2018

Conversation

vitkon
Copy link
Owner

@vitkon vitkon commented Mar 24, 2018

What's this PR do?

  • removes validators from libarary
  • adds a pre-release script
  • adds new examples folder with the replica from sandbox

Where should the reviewer start?

/examples folder and validators.ts

How should this be manually tested?

  • run builds
  • npm start in examples folder

What are the relevant tickets / issues?

#34 #37

@vitkon vitkon requested a review from mattdell March 24, 2018 13:06
@vitkon vitkon force-pushed the issue/34-remove-validators branch from 5978a9c to f35066f Compare March 24, 2018 13:11
@vitkon vitkon force-pushed the issue/34-remove-validators branch from f35066f to b09351d Compare March 24, 2018 13:23
@codecov-io
Copy link

codecov-io commented Mar 24, 2018

Codecov Report

Merging #47 into master will decrease coverage by 1.48%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #47      +/-   ##
==========================================
- Coverage   84.61%   83.13%   -1.49%     
==========================================
  Files           4        3       -1     
  Lines          91       83       -8     
  Branches       16       14       -2     
==========================================
- Hits           77       69       -8     
  Misses          8        8              
  Partials        6        6
Impacted Files Coverage Δ
src/validators.ts 80% <100%> (-8.89%) ⬇️

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 e10761e...b09351d. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.04%) to 83.761% when pulling b09351d on issue/34-remove-validators into e10761e on master.

@mattdell
Copy link
Collaborator

I get

/Users/mdell/Repos/form-container/examples/src/validators.ts
(2,26): Could not find a declaration file for module 'validator/lib/isEmail'. '/Users/mdell/Repos/form-container/examples/node_modules/validator/lib/isEmail.js' implicitly has an 'any' type.
  Try `npm install @types/validator` if it exists or add a new declaration (.d.ts) file containing `declare module 'validator';`

Installing the package fixes it and I can finally see the examples project! 🎉

Worth adding this build to CI as part of this PR?

/>
</CardContent>
<CardActions>
<Button fullWidth={true} type="submit" color="primary">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thoughts on a disabled attribute?

disabled={!isValid}

Copy link
Owner Author

Choose a reason for hiding this comment

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

yeah, let's add it in

@vitkon vitkon force-pushed the issue/34-remove-validators branch 2 times, most recently from bce846d to 6fd4e31 Compare March 24, 2018 16:54
@vitkon vitkon force-pushed the issue/34-remove-validators branch from 6fd4e31 to 309031e Compare March 24, 2018 16:59
@vitkon
Copy link
Owner Author

vitkon commented Mar 24, 2018

  • added disabled attribute to the button
  • added build for examples folder

@mattdell mattdell merged commit d6ead08 into master Mar 25, 2018
@mattdell mattdell deleted the issue/34-remove-validators branch March 25, 2018 09:24
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.

None yet

4 participants