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

refactor(shared-data): port shared-data JS source to TypeScript #7731

Merged
merged 18 commits into from
May 3, 2021

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Apr 27, 2021

Overview

This PR converts the JS source of shared-data to TypeScript.

Current status:

  • Run flow-to-ts on Flow source
  • Fix up errors and formatting weirdness
  • Ensure shared-data project is hooked into other TS projects properly

Changelog

  • refactor(shared-data): port shared-data JS source to TypeScript

Review requests

Please check carefully for any source-code changes

Risk assessment

Medium, shared-data is pretty important! Please be on the lookup for unintentional source-code changes in the review. This PR should be limited to type and style change only

@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

❗ No coverage uploaded for pull request base (edge@e8f2ae9). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #7731   +/-   ##
=======================================
  Coverage        ?   82.77%           
=======================================
  Files           ?      326           
  Lines           ?    21425           
  Branches        ?        0           
=======================================
  Hits            ?    17735           
  Misses          ?     3690           
  Partials        ?        0           

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 e8f2ae9...c8e3961. Read the comment docs.

@mcous mcous marked this pull request as ready for review April 28, 2021 01:42
@mcous mcous requested a review from a team as a code owner April 28, 2021 01:42
@mcous mcous requested a review from a team April 28, 2021 01:42
@mcous mcous requested review from a team as code owners April 28, 2021 01:42
@mcous mcous requested review from a team, sanni-t, b-cooper, Kadee80 and shlokamin and removed request for a team April 28, 2021 01:42
Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

Awesome 💎

shared-data/js/helpers/__tests__/wellSets.test.ts Outdated Show resolved Hide resolved
shared-data/js/modules.ts Outdated Show resolved Hide resolved
shared-data/js/pipettes.ts Outdated Show resolved Hide resolved
shared-data/js/pipettes.ts Outdated Show resolved Hide resolved
shared-data/js/types.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,19 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wanted to call this out because it's a little confusing!

shared-data now has two separate tsconfig files: one for TS, and one for JSON. Having a separate config for JSON allows us to ensure the JSON files are output to lib along with the rest of the .d.ts files, allowing us to use stuff from JSON, like object keys, directly in TypeScript types

Copy link
Contributor

@X-sam X-sam left a comment

Choose a reason for hiding this comment

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

LGTM

@mcous mcous merged commit 11c8328 into edge May 3, 2021
@mcous mcous deleted the shared_data_port-to-ts branch May 3, 2021 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants