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: make amplify prompts dependency explicit, lint errors #10007

Merged
merged 4 commits into from
Mar 17, 2022

Conversation

jhockett
Copy link
Contributor

@jhockett jhockett commented Mar 16, 2022

Description of changes

While working on #9936, I started fixing lint errors which spread to multiple files that were unrelated to overall changes I was making. Separating some of the changes into this PR for review-ability.

Issue #, if available

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes

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

@jhockett jhockett requested a review from a team as a code owner March 16, 2022 22:49
@edwardfoyle
Copy link
Contributor

edwardfoyle commented Mar 17, 2022

Looks like there's a bug in the PR lint check script where it's returning a 0 exit code even when there are errors: https://app.circleci.com/pipelines/github/aws-amplify/amplify-cli/7043/workflows/ed2d3288-0116-4bf9-a011-4f748c2e9151/jobs/188647/parallel-runs/0/steps/0-103
If you want to fix that as part of this PR you can, otherwise just check the errors and I'll fix the script tomorrow

@codecov-commenter
Copy link

Codecov Report

Merging #10007 (29759bc) into master (7f9f072) will not change coverage.
The diff coverage is 57.14%.

@@           Coverage Diff           @@
##           master   #10007   +/-   ##
=======================================
  Coverage   54.08%   54.08%           
=======================================
  Files         837      837           
  Lines       46340    46340           
  Branches     9885     9885           
=======================================
  Hits        25065    25065           
  Misses      19281    19281           
  Partials     1994     1994           
Impacted Files Coverage Δ
packages/amplify-cli-core/src/index.ts 100.00% <ø> (ø)
...ify-cli/src/extensions/amplify-helpers/get-tags.ts 82.35% <57.14%> (+1.10%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@@ -219,14 +283,17 @@ export interface ProviderContext {
projectName: string;
}

/**
*
Copy link
Contributor

Choose a reason for hiding this comment

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

does the fixer just drop these empty blocks in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I didn't comment in some places because I wasn't sure I understood the full context

@jhockett jhockett merged commit 66cdc06 into aws-amplify:master Mar 17, 2022
@jhockett jhockett deleted the lint-fixes branch March 17, 2022 19:20
@github-actions github-actions bot added the referenced-in-release Issues referenced in a published release changelog label Mar 23, 2022
@github-actions
Copy link

👋 Hi, this pull request was referenced in the v7.6.26 release!

Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v7.6.26.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
referenced-in-release Issues referenced in a published release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants