Skip to content
This repository was archived by the owner on Dec 10, 2021. It is now read-only.

Revert "Handle BigNumber conversions in JSON properly (without loss of precision) (#71)" #126

Merged
merged 3 commits into from
Apr 2, 2019

Conversation

xtinec
Copy link
Contributor

@xtinec xtinec commented Mar 29, 2019

This reverts commit e386612.

🐛 Bug Fix

@xtinec xtinec requested a review from a team as a code owner March 29, 2019 05:20
chartType: string;
overrideTransformProps?: TransformProps;
}) => LoadableRenderer<RenderProps, LoadedModules> | (() => null);
processChartProps: (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kristw this is odd -- my linter locally would convert it to this formatting when running git commit but CI doesn't like this new format. 🤔 Do you guys have this issue this locally?

Copy link
Contributor

Choose a reason for hiding this comment

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

Umm, I don't have this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did encounter this issue a couple times, was inconsistent tho 🤔

@xtinec xtinec force-pushed the xtinec--revert-big-number branch from 7f49eb2 to 3cc7069 Compare March 31, 2019 06:14
@xtinec xtinec added the #bug Something isn't working label Mar 31, 2019
@xtinec xtinec requested a review from soboko March 31, 2019 06:16
@ghost
Copy link

ghost commented Mar 31, 2019

There were the following issues with this Pull Request

  • Commit: d1aa5ab
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@xtinec xtinec force-pushed the xtinec--revert-big-number branch 3 times, most recently from f02d176 to 0c4c187 Compare April 1, 2019 17:29
const JSONbig: JSONbig;
export = JSONbig;
}
declare module 'fetch-mock';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you don't want to add this back since we now have the types as a dep. not sure if that's affecting the TS build.

@xtinec xtinec force-pushed the xtinec--revert-big-number branch 2 times, most recently from 94faf6f to 0be02c8 Compare April 1, 2019 23:22
@xtinec xtinec force-pushed the xtinec--revert-big-number branch from 0be02c8 to f1219e1 Compare April 1, 2019 23:52
@@ -9,8 +9,11 @@ const defaultMockLoadFormData = jest.fn(allProps => Promise.resolve(allProps.for

// coerce here else get: Type 'Mock<Promise<any>, []>' is not assignable to type 'Mock<Promise<any>, any[]>'
let mockLoadFormData = defaultMockLoadFormData as jest.Mock<Promise<any>, any>;
const mockLoadDatasource = jest.fn(datasource => Promise.resolve(datasource));
const mockLoadQueryData = jest.fn(input => Promise.resolve(input));
const mockLoadDatasource = jest.fn(datasource => Promise.resolve(datasource)) as jest.Mock<
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These fix the type errors seen in previous builds and on master.

@xtinec xtinec force-pushed the xtinec--revert-big-number branch from a583cd4 to f66ff8c Compare April 2, 2019 08:18
@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@5aa383e). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #126   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?     76           
  Lines             ?    966           
  Branches          ?    232           
=======================================
  Hits              ?    966           
  Misses            ?      0           
  Partials          ?      0
Impacted Files Coverage Δ
...uperset-ui-connection/src/callApi/parseResponse.ts 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 5aa383e...8e69917. Read the comment docs.

@ghost
Copy link

ghost commented Apr 2, 2019

There were the following issues with this Pull Request

  • Commit: b4201be
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@xtinec xtinec force-pushed the xtinec--revert-big-number branch from b4201be to fe356bd Compare April 2, 2019 21:08
@xtinec xtinec requested review from kristw and williaster and removed request for kristw April 2, 2019 21:21
@xtinec xtinec added the reviewable Ready for review label Apr 2, 2019
Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Woot! sorry for all the issues with TS on this one 💔

@xtinec
Copy link
Contributor Author

xtinec commented Apr 2, 2019

@kristw @williaster No worries. Happy to fix them. Thanks for the quick turn-around on these! ❤️

@xtinec xtinec merged commit a53b2af into master Apr 2, 2019
@delete-merged-branch delete-merged-branch bot deleted the xtinec--revert-big-number branch April 2, 2019 21:32
kristw pushed a commit that referenced this pull request Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
#bug Something isn't working reviewable Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants