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(js-api-clients): move js-api-clients to the opentrons repo #8515

Merged
merged 21 commits into from
Oct 14, 2021

Conversation

smb2268
Copy link
Contributor

@smb2268 smb2268 commented Oct 12, 2021

Overview

Move js-api-clients to the /opentrons monorepo to allow for use of shared data types

Changelog

Moved all js-api-clients files to monorepo
Added necessary dev dependencies to root package.json
Fixed linting & TS errors
Added CI yaml file for js-api-clients

Review requests

Ensure existing example-app is still working as expected

Risk assessment

Low

@smb2268 smb2268 self-assigned this Oct 12, 2021
@smb2268 smb2268 requested review from a team as code owners October 12, 2021 19:18
@codecov
Copy link

codecov bot commented Oct 12, 2021

Codecov Report

Merging #8515 (ffbf5a2) into edge (5600948) will decrease coverage by 0.00%.
The diff coverage is 97.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #8515      +/-   ##
==========================================
- Coverage   74.21%   74.20%   -0.01%     
==========================================
  Files        1691     1704      +13     
  Lines       45502    45805     +303     
  Branches     4619     4707      +88     
==========================================
+ Hits        33768    33989     +221     
- Misses      10941    11009      +68     
- Partials      793      807      +14     
Flag Coverage Δ
app 71.45% <ø> (+0.27%) ⬆️
components 47.23% <ø> (+0.44%) ⬆️
labware-library 50.22% <ø> (ø)
protocol-designer 43.97% <ø> (-0.18%) ⬇️
react-api-client 97.29% <97.29%> (?)
step-generation 90.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
react-api-client/src/api/ApiClientProvider.tsx 50.00% <50.00%> (ø)
react-api-client/src/api/ApiHostProvider.tsx 100.00% <100.00%> (ø)
react-api-client/src/api/useHost.ts 100.00% <100.00%> (ø)
react-api-client/src/health/useHealth.ts 100.00% <100.00%> (ø)
...act-api-client/src/sessions/useAllSessionsQuery.ts 100.00% <100.00%> (ø)
...pi-client/src/sessions/useCreateSessionMutation.ts 100.00% <100.00%> (ø)
...t-api-client/src/sessions/useEnsureBasicSession.ts 100.00% <100.00%> (ø)
react-api-client/src/sessions/useSessionQuery.ts 100.00% <100.00%> (ø)
...-api-client/src/sessions/useSessionsByTypeQuery.ts 100.00% <100.00%> (ø)
components/src/testing/utils/matchers.ts 58.82% <0.00%> (-24.51%) ⬇️
... and 24 more

Copy link
Contributor

@mcous mcous left a comment

Choose a reason for hiding this comment

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

I think double nesting these projects inside js-api-clients may be confusing, and I'd be softly in favor of not doing that. Otherwise, various comments about integrating the config of these configs into the monorepo more consistently

js-api-clients/LICENSE Outdated Show resolved Hide resolved
js-api-clients/README.md Outdated Show resolved Hide resolved
js-api-clients/babel.config.js Outdated Show resolved Hide resolved
js-api-clients/example-app/README.md Outdated Show resolved Hide resolved
js-api-clients/jest.config.js Outdated Show resolved Hide resolved
js-api-clients/package.json Outdated Show resolved Hide resolved
js-api-clients/tsconfig.json Outdated Show resolved Hide resolved
js-api-clients/yarn.lock Outdated Show resolved Hide resolved
Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

Approving for now, we need to come back and address the hacks I added once we upgrade to react 16.14, link to slack thread

Copy link
Contributor

@mcous mcous left a comment

Choose a reason for hiding this comment

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

LGTM. A few final cleanup items, but nothing major

js-api-client/package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
react-api-client/Makefile Outdated Show resolved Hide resolved
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.

☑️

@smb2268 smb2268 merged commit c67eb8b into edge Oct 14, 2021
@smb2268 smb2268 deleted the js-api-client_move branch October 14, 2021 21:18
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