This repository has been archived by the owner. It is now read-only.

Add the ability to set the default apiCheckInstance for all types #410

Closed
kentcdodds opened this Issue Jul 29, 2015 · 3 comments

Comments

Projects
None yet
1 participant
@kentcdodds
Member

kentcdodds commented Jul 29, 2015

Something like formlyConfig.extras.defaultTypeApiCheckInstance. So I don't have to specify apiCheckInstance on all of my types and wrappers.

@kentcdodds

This comment has been minimized.

Show comment
Hide comment
@kentcdodds

kentcdodds Aug 3, 2015

Member

As before this one is for first-timers-only. That means that I will only accept a PR for this one from someone who's never contributed to open source before. This one is particularly easy (but don't make that statement make you feel bad if you have a hard time with it, there's more to contributing to open source than changing lines of code, especially if it's your first time). I'll hold your hand through this if you need me to. :-) Here are the steps to get a PR merged here.

  • Watch this video to learn what you need to do to get things setup
  • Go to this test and change describe.skip to describe.only
  • Run $ npm test
  • Notice that it's failing
  • Go to this line. This is where formly decides which instance of apiCheck to use to issue the warning. We need to tell formly to default to the field's instance (the one that's passed in and is called apiCheckInstance), then the formlyConfig.extras.apiCheckInstance, then the formlyApiCheck instance. Right now it's missing the formlyConfig.extras.apiCheckInstance. I would just add it as another || between the first and second one on the same line.
  • Update the eslint comment here to allow for a complexity of 9 (instead of 8) because you're adding another branch of complexity. Don't worry, it's covered, and I think that the code is fairly easy to understand. If you want, you could refactor it to make it even better.
  • Once the test is passing, change the describe.only to describe so all the tests run.
  • Make sure that all the tests pass
  • Commit your changes to the src/ directory mentioning issue #343 (note it may take a second... it runs all the tests...)
  • Push your changes to your fork, create a PR, get merged, celebrate 🎉

I'm happy to hold your hand through this if you need help. Catch me on gitter.

Member

kentcdodds commented Aug 3, 2015

As before this one is for first-timers-only. That means that I will only accept a PR for this one from someone who's never contributed to open source before. This one is particularly easy (but don't make that statement make you feel bad if you have a hard time with it, there's more to contributing to open source than changing lines of code, especially if it's your first time). I'll hold your hand through this if you need me to. :-) Here are the steps to get a PR merged here.

  • Watch this video to learn what you need to do to get things setup
  • Go to this test and change describe.skip to describe.only
  • Run $ npm test
  • Notice that it's failing
  • Go to this line. This is where formly decides which instance of apiCheck to use to issue the warning. We need to tell formly to default to the field's instance (the one that's passed in and is called apiCheckInstance), then the formlyConfig.extras.apiCheckInstance, then the formlyApiCheck instance. Right now it's missing the formlyConfig.extras.apiCheckInstance. I would just add it as another || between the first and second one on the same line.
  • Update the eslint comment here to allow for a complexity of 9 (instead of 8) because you're adding another branch of complexity. Don't worry, it's covered, and I think that the code is fairly easy to understand. If you want, you could refactor it to make it even better.
  • Once the test is passing, change the describe.only to describe so all the tests run.
  • Make sure that all the tests pass
  • Commit your changes to the src/ directory mentioning issue #343 (note it may take a second... it runs all the tests...)
  • Push your changes to your fork, create a PR, get merged, celebrate 🎉

I'm happy to hold your hand through this if you need help. Catch me on gitter.

bmacheski added a commit to bmacheski/angular-formly that referenced this issue Aug 3, 2015

kentcdodds added a commit that referenced this issue Aug 3, 2015

@kentcdodds

This comment has been minimized.

Show comment
Hide comment
Member

kentcdodds commented Aug 3, 2015

Thanks @bmacheski!

@kentcdodds kentcdodds closed this Aug 3, 2015

@kentcdodds

This comment has been minimized.

Show comment
Hide comment
@kentcdodds

kentcdodds Aug 3, 2015

Member

This has been released in angular-formly@6.23.0 :-)

Member

kentcdodds commented Aug 3, 2015

This has been released in angular-formly@6.23.0 :-)

@kentcdodds kentcdodds referenced this issue Aug 4, 2015

Merged

Fix for #410 #415

@hollenberry hollenberry referenced this issue in github/training-kit Dec 6, 2016

Closed

Open Source Community Updates #415

13 of 14 tasks complete

@cannawen cannawen referenced this issue in cannawen/metric_units_reddit_bot Oct 7, 2017

Closed

Discussion - Improve new contributor experience #70

7 of 8 tasks complete
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.