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

fix: optimize mock package imports #6455

Merged
merged 2 commits into from
Jan 26, 2021

Conversation

edwardfoyle
Copy link
Contributor

Issue #, if available:
#5103

Description of changes:
There were some circular imports when importing the mock package causing the import of that package to take ~5 seconds. This PR fixes these circular imports as well as migrates the command .js files to .ts files

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@edwardfoyle edwardfoyle requested a review from a team as a code owner January 21, 2021 22:55
@edwardfoyle edwardfoyle requested review from jhockett and yuth and removed request for a team January 21, 2021 22:55
@codecov
Copy link

codecov bot commented Jan 21, 2021

Codecov Report

Merging #6455 (4f2384c) into master (e28035a) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6455      +/-   ##
==========================================
- Coverage   57.30%   57.25%   -0.06%     
==========================================
  Files         474      479       +5     
  Lines       21669    21689      +20     
  Branches     4306     4310       +4     
==========================================
  Hits        12418    12418              
- Misses       8370     8390      +20     
  Partials      881      881              
Impacted Files Coverage Δ
packages/amplify-cli-core/src/index.ts 94.73% <ø> (ø)
...es/amplify-util-mock/src/CFNParser/field-parser.ts 0.00% <0.00%> (ø)
packages/amplify-util-mock/src/CFNParser/index.ts 0.00% <ø> (ø)
...ages/amplify-util-mock/src/amplify-plugin-index.ts 0.00% <0.00%> (ø)
...ackages/amplify-util-mock/src/commands/mock/api.ts 0.00% <0.00%> (ø)
...es/amplify-util-mock/src/commands/mock/function.ts 0.00% <0.00%> (ø)
...ckages/amplify-util-mock/src/commands/mock/help.ts 0.00% <0.00%> (ø)
...ckages/amplify-util-mock/src/commands/mock/mock.ts 0.00% <0.00%> (ø)
...ges/amplify-util-mock/src/commands/mock/storage.ts 0.00% <0.00%> (ø)
packages/amplify-util-mock/src/index.ts 0.00% <ø> (ø)
... and 5 more

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 e28035a...4f2384c. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Jan 21, 2021

This pull request fixes 1 alert when merging 0bc9605 into 0530d1a - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

Copy link
Contributor

@jhockett jhockett left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of minor nits

packages/amplify-util-mock/src/commands/mock/api.ts Outdated Show resolved Hide resolved
packages/amplify-util-mock/src/commands/mock/mock.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2021

This pull request fixes 1 alert when merging 4f2384c into 75f5ace - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@jhockett jhockett merged commit 1b64a14 into aws-amplify:master Jan 26, 2021
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants