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

Add type definitions for redux-duck #37624

Merged
merged 1 commit into from
Aug 21, 2019

Conversation

cyberixae
Copy link
Contributor

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If adding a new definition:

  • The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
  • If this is for an NPM package, match the name. If not, do not conflict with the name of an NPM package.
  • Create it with dts-gen --dt, not by basing it on an existing project.
  • tslint.json should be present, and tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

@cyberixae
Copy link
Contributor Author

cyberixae commented Aug 14, 2019

The types depend on redux and flux-standard-action. The tests pass if I place the dependencies above DefinitelyTyped director before running npm test. Both of these packages ship their own type definitions. The redux package is already in the whitelist. Not sure how to request addition of flux-standard-action.

@typescript-bot typescript-bot added this to Needs Author Attention in Pull Request Status Board Aug 14, 2019
@typescript-bot typescript-bot added New Definition This PR creates a new definition package. The Travis CI build failed labels Aug 14, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Aug 14, 2019

@cyberixae Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

@typescript-bot
Copy link
Contributor

typescript-bot commented Aug 14, 2019

@cyberixae The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@cyberixae
Copy link
Contributor Author

I asked the flux-standard-action developers if they are interested in migrating their types to DefinitelyTyped. Still not sure why redux fails though. Do I need to do some trick to install the whitelisted packages?

@typescript-bot
Copy link
Contributor

@cyberixae I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

@typescript-bot
Copy link
Contributor

typescript-bot commented Aug 21, 2019

@cyberixae The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@cyberixae
Copy link
Contributor Author

I added a package.json with dependencty to redux and created a minimal mock for flux-standard-action types and documented why the mock was created.

@typescript-bot
Copy link
Contributor

👋 Hi there! I’ve run some quick performance metrics against master and your PR. This is still an experiment, so don’t panic if I say something crazy! I’m still learning how to interpret these metrics.

Let’s review the numbers, shall we?

These typings are for a package that doesn’t yet exist on master, so I don’t have anything to compare against yet! In the future, I’ll be able to compare PRs to redux-duck with its source on master.

Comparison details 📊
Batch compilation
Type count 2733
Assignability cache size 334
Subtype cache size 0
Identity cache size 0
Language service measurements
Samples taken 131
Identifiers in tests 131
getCompletionsAtPosition
    Mean duration (ms) 103.1
    Median duration (ms) 155.2
    Mean CV 25.6%
    Worst duration (ms) 181.0
    Worst identifier state
getQuickInfoAtPosition
    Mean duration (ms) 100.0
    Median duration (ms) 80.1
    Mean CV 23.1%
    Worst duration (ms) 153.0
    Worst identifier RETURN_HOME
System information
Node version v10.16.1
CPU count 2
CPU speed 2.294 GHz
CPU model Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz
CPU Architecture x64
Memory 6.8 GiB
Platform linux
Release 4.15.0-1052-azure

If you have any questions or comments about me, you can ping @andrewbranch. Have a nice day!

@typescript-bot typescript-bot moved this from Needs Author Attention to Review in Pull Request Status Board Aug 21, 2019
@sheetalkamat sheetalkamat merged commit 4b96649 into DefinitelyTyped:master Aug 21, 2019
Pull Request Status Board automation moved this from Review to Done Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Definition This PR creates a new definition package.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants